Message ID | 38e5ef70b494a5e21fdb89b2105e9a87fd410f7c.1350677362.git.jbaron@redhat.com |
---|---|
State | New |
Headers | show |
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
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); + } + filename = qemu_find_file_in_dir(type, name, data_dir); + if (!filename) { + filename = qemu_find_file_in_dir(type, name, data_dir_fallback); + } + return filename; +} + static int device_help_func(QemuOpts *opts, void *opaque) { return qdev_device_help(opts); @@ -3359,12 +3375,10 @@ int main(int argc, char **argv, char **envp) /* If no data_dir is specified then try to find it relative to the executable path. */ - if (!data_dir) { - data_dir = os_find_datadir(argv[0]); - } + data_dir_fallback = os_find_datadir(argv[0]); /* If all else fails use the install path specified when building. */ - if (!data_dir) { - data_dir = CONFIG_QEMU_DATADIR; + if (!data_dir_fallback) { + data_dir_fallback = CONFIG_QEMU_DATADIR; } /*