| Message ID | 20210807214242.82385-13-imp@bsdimp.com |
|---|---|
| State | New |
| Headers | show |
| Series | bsd-user updates to run hello world | expand |
On 8/7/21 11:42 AM, Warner Losh wrote: > + path = g_strdup(p); > + if (path == NULL) { Only returns null when the input is null, which you've already eliminated. > +static bool find_in_path(char *path, const char *filename, char *retpath, > + size_t rpsize) > +{ > + const char *d; > + > + while ((d = strsep(&path, ":")) != NULL) { > + if (*d == '\0') { > + d = "."; > + } > + if (snprintf(retpath, rpsize, "%s/%s", d, filename) >= (int)rpsize) { > + continue; > + } > + if (is_there((const char *)retpath)) { > + return true; > + } > + } > + return false; Hmm. Fixed size retpath buffer isn't ideal. Any reason not to use g_find_program_in_path? I note that we don't search the path at all in linux-user/. r~
On Sat, Aug 7, 2021 at 10:11 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/7/21 11:42 AM, Warner Losh wrote: > > + path = g_strdup(p); > > + if (path == NULL) { > > Only returns null when the input is null, which you've already eliminated. > > > +static bool find_in_path(char *path, const char *filename, char *retpath, > > + size_t rpsize) > > +{ > > + const char *d; > > + > > + while ((d = strsep(&path, ":")) != NULL) { > > + if (*d == '\0') { > > + d = "."; > > + } > > + if (snprintf(retpath, rpsize, "%s/%s", d, filename) >= (int)rpsize) { > > + continue; > > + } > > + if (is_there((const char *)retpath)) { > > + return true; > > + } > > + } > > + return false; > > Hmm. Fixed size retpath buffer isn't ideal. > Any reason not to use g_find_program_in_path? > g_find_program_in_path may work well here, as well... > I note that we don't search the path at all in linux-user/. > IIRC imgact_binmisc will have the resolved path but preserve argv as it should have been were it not emulated, so we have to re-evaluate the PATH search here because we try to be faithful to the context. Thanks, Kyle Evans
On Sat, Aug 7, 2021 at 11:49 PM Kyle Evans <kevans@freebsd.org> wrote: > On Sat, Aug 7, 2021 at 10:11 PM Richard Henderson > <richard.henderson@linaro.org> wrote: > > > > On 8/7/21 11:42 AM, Warner Losh wrote: > > > + path = g_strdup(p); > > > + if (path == NULL) { > > > > Only returns null when the input is null, which you've already > eliminated. > > > > > +static bool find_in_path(char *path, const char *filename, char > *retpath, > > > + size_t rpsize) > > > +{ > > > + const char *d; > > > + > > > + while ((d = strsep(&path, ":")) != NULL) { > > > + if (*d == '\0') { > > > + d = "."; > > > + } > > > + if (snprintf(retpath, rpsize, "%s/%s", d, filename) >= > (int)rpsize) { > > > + continue; > > > + } > > > + if (is_there((const char *)retpath)) { > > > + return true; > > > + } > > > + } > > > + return false; > > > > Hmm. Fixed size retpath buffer isn't ideal. > > Any reason not to use g_find_program_in_path? > > > > g_find_program_in_path may work well here, as well... > Quite well: 1 file changed, 5 insertions(+), 42 deletions(-) so a nice reduction in code when the unnecessary stuff is removed. I'll have that in v2 of the series. > I note that we don't search the path at all in linux-user/. > > > > IIRC imgact_binmisc will have the resolved path but preserve argv as > it should have been were it not emulated, so we have to re-evaluate > the PATH search here because we try to be faithful to the context. > I just looked at the imgact_binmisc code, and that's what it does, so that sounds reasonable to me as well. Warner
diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c index 379015c744..f8030d72bc 100644 --- a/bsd-user/bsdload.c +++ b/bsd-user/bsdload.c @@ -139,17 +139,88 @@ abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp, return sp; } +static bool is_there(const char *candidate) +{ + struct stat fin; + + /* XXX work around access(2) false positives for superuser */ + if (access(candidate, X_OK) == 0 && stat(candidate, &fin) == 0 && + S_ISREG(fin.st_mode) && (getuid() != 0 || + (fin.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) != 0)) { + return true; + } + + return false; +} + +static bool find_in_path(char *path, const char *filename, char *retpath, + size_t rpsize) +{ + const char *d; + + while ((d = strsep(&path, ":")) != NULL) { + if (*d == '\0') { + d = "."; + } + if (snprintf(retpath, rpsize, "%s/%s", d, filename) >= (int)rpsize) { + continue; + } + if (is_there((const char *)retpath)) { + return true; + } + } + return false; +} + int loader_exec(const char *filename, char **argv, char **envp, struct target_pt_regs *regs, struct image_info *infop, struct bsd_binprm *bprm) { + char *p, *path = NULL, fullpath[PATH_MAX]; int retval, i; bprm->p = TARGET_PAGE_SIZE * MAX_ARG_PAGES; for (i = 0; i < MAX_ARG_PAGES; i++) { /* clear page-table */ bprm->page[i] = NULL; } - retval = open(filename, O_RDONLY); + + if (strchr(filename, '/') != NULL) { + path = realpath(filename, fullpath); + if (path == NULL) { + /* Failed to resolve. */ + return -1; + } + if (!is_there(path)) { + return -1; + } + retval = open(path, O_RDONLY); + bprm->fullpath = g_strdup(path); + } else { + p = getenv("PATH"); + if (p == NULL) { + return -1; + } + + path = g_strdup(p); + if (path == NULL) { + fprintf(stderr, "Out of memory\n"); + return -1; + } + + if (!find_in_path(path, filename, fullpath, sizeof(fullpath))) { + return -1; + } + retval = open(fullpath, O_RDONLY); + bprm->fullpath = g_strdup(fullpath); + + g_free(path); + } + + /* bprm->fullpath must be populated. */ + if (bprm->fullpath == NULL) { + fprintf(stderr, "Out of memory\n"); + return -1; + } if (retval < 0) { return retval; } diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h index 5237e35f9c..6b601ce4b5 100644 --- a/bsd-user/qemu.h +++ b/bsd-user/qemu.h @@ -124,7 +124,8 @@ struct bsd_binprm { int argc, envc; char **argv; char **envp; - char *filename; /* Name of binary */ + char *filename; /* (Given) Name of binary */ + char *fullpath; /* Full path of binary */ }; void do_init_thread(struct target_pt_regs *regs, struct image_info *infop);