[v3,22/26] Add a fallback bios file search, if -L fails.

From: Jason Baron <jbaron@redhat.com>

If -L <dir> is specified, and qemu does not find the bios file in <dir>, then
the search fails. Add infrastructure such that the search will continue in
the default paths, if not found in the -L path.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 vl.c |   36 +++++++++++++++++++++++++-----------
 1 files changed, 25 insertions(+), 11 deletions(-)

On 20.10.2012 00:43, Jason Baron wrote:
> From: Jason Baron <jbaron@redhat.com>
> 
> If -L <dir> is specified, and qemu does not find the bios file in <dir>, then
> the search fails. Add infrastructure such that the search will continue in
> the default paths, if not found in the -L path.
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  vl.c |   36 +++++++++++++++++++++++++-----------
>  1 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 6b1e546..2a2d217 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -177,6 +177,7 @@ int main(int argc, char **argv)
>  #define MAX_VIRTIO_CONSOLES 1
>  
>  static const char *data_dir;
> +static const char *data_dir_fallback;
>  const char *bios_name = NULL;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
>  DisplayType display_type = DT_DEFAULT;
> @@ -1892,16 +1893,16 @@ static int balloon_parse(const char *arg)
>      return -1;
>  }
>  
> -char *qemu_find_file(int type, const char *name)
> +static char *qemu_find_file_in_dir(int type, const char *name, const char *dir)
>  {
>      int len;
>      const char *subdir;
>      char *buf;
>  
> -    /* Try the name as a straight path first */
> -    if (access(name, R_OK) == 0) {
> -        return g_strdup(name);
> +    if (!dir) {
> +        return NULL;
>      }
> +
>      switch (type) {
>      case QEMU_FILE_TYPE_BIOS:
>          subdir = "";
> @@ -1912,9 +1913,9 @@ char *qemu_find_file(int type, const char *name)
>      default:
>          abort();
>      }
> -    len = strlen(data_dir) + strlen(name) + strlen(subdir) + 2;
> +    len = strlen(dir) + strlen(name) + strlen(subdir) + 2;
>      buf = g_malloc0(len);
> -    snprintf(buf, len, "%s/%s%s", data_dir, subdir, name);
> +    snprintf(buf, len, "%s/%s%s", dir, subdir, name);
>      if (access(buf, R_OK)) {
>          g_free(buf);
>          return NULL;
> @@ -1922,6 +1923,21 @@ char *qemu_find_file(int type, const char *name)
>      return buf;
>  }
>  
> +char *qemu_find_file(int type, const char *name)
> +{
> +    char *filename;
> +
> +    /* Try the name as a straight path first */
> +    if (access(name, R_OK) == 0) {
> +        return g_strdup(name);
> +    }

FWIW, this can be a security issue, when a more privileged
user tries to run qemu from trusted path (/usr/bin) in a
directory owned by non-privileged user, to utilize -runas
or somesuch.  I understand it's been this way since the
beginning.

Maybe we can do a bit better here, like (windows systems
aside) this:

    if (strchr(name, '/') && access(name, R_OK) == 0) {...}

Note that -L can be set to "." in order to check in current
directory too.

And going forward, maybe we can treat -L argument as a PATH
(separated by whatever delimiter is okay) instead of just
a single directory?  This way, for example, we can put
various kinds of data (pxe roms, vgabios roms etc) into
different places and qemu will be able to find them all.

I understand this whole thing is not really related to q35
changes, so can be pushed/applied separately.

Thanks,

/mjt

On 21 October 2012 08:26, Michael Tokarev <mjt@tls.msk.ru> wrote:
> On 20.10.2012 00:43, Jason Baron wrote:
>> +char *qemu_find_file(int type, const char *name)
>> +{
>> +    char *filename;
>> +
>> +    /* Try the name as a straight path first */
>> +    if (access(name, R_OK) == 0) {
>> +        return g_strdup(name);
>> +    }
>
> FWIW, this can be a security issue, when a more privileged
> user tries to run qemu from trusted path (/usr/bin) in a
> directory owned by non-privileged user, to utilize -runas
> or somesuch.  I understand it's been this way since the
> beginning.
>
> Maybe we can do a bit better here, like (windows systems
> aside) this:
>
>     if (strchr(name, '/') && access(name, R_OK) == 0) {...}

We used to do that, but it was removed in commit 3178320
because it's inconsistent with how we handle other file
access (like -kernel). The documentation says -bios takes
a filename, so it should just take a filename, with no
weird undocumented restrictions.

If you want qemu not to read files from the current
working directory by default the right fix for that is
probably to make those defaults be "foo.bin in the bios path",
not unqualified "foo.bin".

-- PMM