| Message ID | 20200221202857.126170-1-yuanzi@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Add AT_EXECFN and AT_EXECFD auxval | expand |
On Fri, Feb 21, 2020 at 12:29 PM Lirong Yuan <yuanzi@google.com> wrote: > > This change adds the support for AT_EXECFN and AT_EXECFD auxval. > > Signed-off-by: Lirong Yuan <yuanzi@google.com> > --- > linux-user/elfload.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index f3080a1635..7e0f3042f1 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -1568,7 +1568,7 @@ struct exec > ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) > #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) > > -#define DLINFO_ITEMS 15 > +#define DLINFO_ITEMS 17 > > static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) > { > @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s > return sp; > } > > -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > +static abi_ulong create_elf_tables(struct linux_binprm *bprm, > struct elfhdr *exec, > struct image_info *info, > struct image_info *interp_info) > { > + abi_ulong p = bprm->p; > + int argc = bprm->argc; > + int envc = bprm->envc; > abi_ulong sp; > abi_ulong u_argc, u_argv, u_envp, u_auxv; > int size; > @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); > NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); > NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); > + NEW_AUX_ENT(AT_EXECFN, info->file_string); > + NEW_AUX_ENT(AT_EXECFD, bprm->fd); > > #ifdef ELF_HWCAP2 > NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); > @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) > #endif > } > > - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, > - info, (elf_interpreter ? &interp_info : NULL)); > + bprm->p = create_elf_tables(bprm, &elf_ex, info, > + (elf_interpreter ? &interp_info : NULL)); > info->start_stack = bprm->p; > > /* If we have an interpreter, set that as the program's entry point. > -- > 2.25.0.265.gbab2e86ba0-goog > Friendly ping~ Link to the page for the patch on patchwork: http://patchwork.ozlabs.org/patch/1242331/
Le 21/02/2020 à 21:28, Lirong Yuan a écrit : > This change adds the support for AT_EXECFN and AT_EXECFD auxval. Why do we need AT_EXECFD? AT_EXECFD is normally only used with binfmt_misc so I don't see any use cases for it with QEMU. For AT_EXECFN, according to kernel commit 651910874633 execve filename: document and export via auxiliary vector It sould be like readlink("/proc/self/exe",), and thus I think we should use realpath() like we have in syscall.c for TARGET_NR_readlink: 8843 case TARGET_NR_readlink: ... 8854 char real[PATH_MAX], *temp; 8855 temp = realpath(exec_path, real); ... Thanks, Laurent > > Signed-off-by: Lirong Yuan <yuanzi@google.com> > --- > linux-user/elfload.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index f3080a1635..7e0f3042f1 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -1568,7 +1568,7 @@ struct exec > ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) > #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) > > -#define DLINFO_ITEMS 15 > +#define DLINFO_ITEMS 17 > > static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) > { > @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s > return sp; > } > > -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > +static abi_ulong create_elf_tables(struct linux_binprm *bprm, > struct elfhdr *exec, > struct image_info *info, > struct image_info *interp_info) > { > + abi_ulong p = bprm->p; > + int argc = bprm->argc; > + int envc = bprm->envc; > abi_ulong sp; > abi_ulong u_argc, u_argv, u_envp, u_auxv; > int size; > @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); > NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); > NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); > + NEW_AUX_ENT(AT_EXECFN, info->file_string); > + NEW_AUX_ENT(AT_EXECFD, bprm->fd); > > #ifdef ELF_HWCAP2 > NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); > @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) > #endif > } > > - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, > - info, (elf_interpreter ? &interp_info : NULL)); > + bprm->p = create_elf_tables(bprm, &elf_ex, info, > + (elf_interpreter ? &interp_info : NULL)); > info->start_stack = bprm->p; > > /* If we have an interpreter, set that as the program's entry point. >
On Mon, Mar 2, 2020 at 6:39 AM Laurent Vivier <laurent@vivier.eu> wrote: > > Le 21/02/2020 à 21:28, Lirong Yuan a écrit : > > This change adds the support for AT_EXECFN and AT_EXECFD auxval. > > Why do we need AT_EXECFD? > > AT_EXECFD is normally only used with binfmt_misc so I don't see any use > cases for it with QEMU. > > For AT_EXECFN, according to kernel commit > > 651910874633 execve filename: document and export via auxiliary vector > > It sould be like readlink("/proc/self/exe",), and thus I think we should > use realpath() like we have in syscall.c for TARGET_NR_readlink: > > 8843 case TARGET_NR_readlink: > ... > 8854 char real[PATH_MAX], *temp; > 8855 temp = realpath(exec_path, real); > ... > > Thanks, > Laurent > > > > > Signed-off-by: Lirong Yuan <yuanzi@google.com> > > --- > > linux-user/elfload.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > > index f3080a1635..7e0f3042f1 100644 > > --- a/linux-user/elfload.c > > +++ b/linux-user/elfload.c > > @@ -1568,7 +1568,7 @@ struct exec > > ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) > > #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) > > > > -#define DLINFO_ITEMS 15 > > +#define DLINFO_ITEMS 17 > > > > static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) > > { > > @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s > > return sp; > > } > > > > -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > > +static abi_ulong create_elf_tables(struct linux_binprm *bprm, > > struct elfhdr *exec, > > struct image_info *info, > > struct image_info *interp_info) > > { > > + abi_ulong p = bprm->p; > > + int argc = bprm->argc; > > + int envc = bprm->envc; > > abi_ulong sp; > > abi_ulong u_argc, u_argv, u_envp, u_auxv; > > int size; > > @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > > NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); > > NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); > > NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); > > + NEW_AUX_ENT(AT_EXECFN, info->file_string); > > + NEW_AUX_ENT(AT_EXECFD, bprm->fd); > > > > #ifdef ELF_HWCAP2 > > NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); > > @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) > > #endif > > } > > > > - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, > > - info, (elf_interpreter ? &interp_info : NULL)); > > + bprm->p = create_elf_tables(bprm, &elf_ex, info, > > + (elf_interpreter ? &interp_info : NULL)); > > info->start_stack = bprm->p; > > > > /* If we have an interpreter, set that as the program's entry point. > > > Hi Laurent, I added support for AT_EXECFD because I thought it might be useful to implement all types that getauxval could take as an argument. Would you prefer that it be removed? For AT_EXECFN, there are two questions that we considered: 1) What should it return? Since QEMU is emulating running the guest program, the function should return the file name of the guest program (info->file_string), rather than the QEMU program itself, which we get from qemu_getauxval(AT_EXECFN). 2) Should it return the full path or as is? We tested the behavior of getauxval with a simple test program on Linux, and it turned out that it returned file path as is. For example, $ ./test getauxval(AT_EXECFN): ./test $ /usr/local/home/tmp/test getauxval(AT_EXECFN): /usr/local/home/tmp/test It would seem that the current solution is working as intended. Thanks, Lirong
Le 02/03/2020 à 19:18, Lirong Yuan a écrit : > On Mon, Mar 2, 2020 at 6:39 AM Laurent Vivier <laurent@vivier.eu> wrote: >> >> Le 21/02/2020 à 21:28, Lirong Yuan a écrit : >>> This change adds the support for AT_EXECFN and AT_EXECFD auxval. >> >> Why do we need AT_EXECFD? >> >> AT_EXECFD is normally only used with binfmt_misc so I don't see any use >> cases for it with QEMU. >> >> For AT_EXECFN, according to kernel commit >> >> 651910874633 execve filename: document and export via auxiliary vector >> >> It sould be like readlink("/proc/self/exe",), and thus I think we should >> use realpath() like we have in syscall.c for TARGET_NR_readlink: >> >> 8843 case TARGET_NR_readlink: >> ... >> 8854 char real[PATH_MAX], *temp; >> 8855 temp = realpath(exec_path, real); >> ... >> >> Thanks, >> Laurent >> >>> >>> Signed-off-by: Lirong Yuan <yuanzi@google.com> >>> --- >>> linux-user/elfload.c | 13 +++++++++---- >>> 1 file changed, 9 insertions(+), 4 deletions(-) >>> >>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c >>> index f3080a1635..7e0f3042f1 100644 >>> --- a/linux-user/elfload.c >>> +++ b/linux-user/elfload.c >>> @@ -1568,7 +1568,7 @@ struct exec >>> ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) >>> #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) >>> >>> -#define DLINFO_ITEMS 15 >>> +#define DLINFO_ITEMS 17 >>> >>> static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) >>> { >>> @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s >>> return sp; >>> } >>> >>> -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, >>> +static abi_ulong create_elf_tables(struct linux_binprm *bprm, >>> struct elfhdr *exec, >>> struct image_info *info, >>> struct image_info *interp_info) >>> { >>> + abi_ulong p = bprm->p; >>> + int argc = bprm->argc; >>> + int envc = bprm->envc; >>> abi_ulong sp; >>> abi_ulong u_argc, u_argv, u_envp, u_auxv; >>> int size; >>> @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, >>> NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); >>> NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); >>> NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); >>> + NEW_AUX_ENT(AT_EXECFN, info->file_string); >>> + NEW_AUX_ENT(AT_EXECFD, bprm->fd); >>> >>> #ifdef ELF_HWCAP2 >>> NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); >>> @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) >>> #endif >>> } >>> >>> - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, >>> - info, (elf_interpreter ? &interp_info : NULL)); >>> + bprm->p = create_elf_tables(bprm, &elf_ex, info, >>> + (elf_interpreter ? &interp_info : NULL)); >>> info->start_stack = bprm->p; >>> >>> /* If we have an interpreter, set that as the program's entry point. >>> >> > > Hi Laurent, Hi Lirong, > I added support for AT_EXECFD because I thought it might be useful to > implement all types that getauxval could take as an argument. > Would you prefer that it be removed? I think providing the AT_EXECFD to the target binary could make it think it has been run directly by binfmt_misc (as an interpreter itself), and that is not the case (qemu is run by binfmt_misc and is the interpreter). So I would prefer you remove it. > For AT_EXECFN, there are two questions that we considered: > 1) What should it return? > Since QEMU is emulating running the guest program, the function should > return the file name of the guest program (info->file_string), rather > than the QEMU program itself, which we get from > qemu_getauxval(AT_EXECFN). > > 2) Should it return the full path or as is? > We tested the behavior of getauxval with a simple test program on > Linux, and it turned out that it returned file path as is. For > example, > $ ./test > getauxval(AT_EXECFN): ./test > $ /usr/local/home/tmp/test > getauxval(AT_EXECFN): /usr/local/home/tmp/test If you have compared the result with the real one it's fine for me. Resend tou patch without the AT_EXECFD part and it will be good. Thanks, Laurent
On Mon, Mar 2, 2020 at 10:31 AM Laurent Vivier <laurent@vivier.eu> wrote: > > Le 02/03/2020 à 19:18, Lirong Yuan a écrit : > > On Mon, Mar 2, 2020 at 6:39 AM Laurent Vivier <laurent@vivier.eu> wrote: > >> > >> Le 21/02/2020 à 21:28, Lirong Yuan a écrit : > >>> This change adds the support for AT_EXECFN and AT_EXECFD auxval. > >> > >> Why do we need AT_EXECFD? > >> > >> AT_EXECFD is normally only used with binfmt_misc so I don't see any use > >> cases for it with QEMU. > >> > >> For AT_EXECFN, according to kernel commit > >> > >> 651910874633 execve filename: document and export via auxiliary vector > >> > >> It sould be like readlink("/proc/self/exe",), and thus I think we should > >> use realpath() like we have in syscall.c for TARGET_NR_readlink: > >> > >> 8843 case TARGET_NR_readlink: > >> ... > >> 8854 char real[PATH_MAX], *temp; > >> 8855 temp = realpath(exec_path, real); > >> ... > >> > >> Thanks, > >> Laurent > >> > >>> > >>> Signed-off-by: Lirong Yuan <yuanzi@google.com> > >>> --- > >>> linux-user/elfload.c | 13 +++++++++---- > >>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c > >>> index f3080a1635..7e0f3042f1 100644 > >>> --- a/linux-user/elfload.c > >>> +++ b/linux-user/elfload.c > >>> @@ -1568,7 +1568,7 @@ struct exec > >>> ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) > >>> #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) > >>> > >>> -#define DLINFO_ITEMS 15 > >>> +#define DLINFO_ITEMS 17 > >>> > >>> static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) > >>> { > >>> @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s > >>> return sp; > >>> } > >>> > >>> -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > >>> +static abi_ulong create_elf_tables(struct linux_binprm *bprm, > >>> struct elfhdr *exec, > >>> struct image_info *info, > >>> struct image_info *interp_info) > >>> { > >>> + abi_ulong p = bprm->p; > >>> + int argc = bprm->argc; > >>> + int envc = bprm->envc; > >>> abi_ulong sp; > >>> abi_ulong u_argc, u_argv, u_envp, u_auxv; > >>> int size; > >>> @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, > >>> NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); > >>> NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); > >>> NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); > >>> + NEW_AUX_ENT(AT_EXECFN, info->file_string); > >>> + NEW_AUX_ENT(AT_EXECFD, bprm->fd); > >>> > >>> #ifdef ELF_HWCAP2 > >>> NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); > >>> @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) > >>> #endif > >>> } > >>> > >>> - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, > >>> - info, (elf_interpreter ? &interp_info : NULL)); > >>> + bprm->p = create_elf_tables(bprm, &elf_ex, info, > >>> + (elf_interpreter ? &interp_info : NULL)); > >>> info->start_stack = bprm->p; > >>> > >>> /* If we have an interpreter, set that as the program's entry point. > >>> > >> > > > > Hi Laurent, > > Hi Lirong, > > > I added support for AT_EXECFD because I thought it might be useful to > > implement all types that getauxval could take as an argument. > > Would you prefer that it be removed? > > I think providing the AT_EXECFD to the target binary could make it think > it has been run directly by binfmt_misc (as an interpreter itself), and > that is not the case (qemu is run by binfmt_misc and is the interpreter). > > So I would prefer you remove it. > > > For AT_EXECFN, there are two questions that we considered: > > 1) What should it return? > > Since QEMU is emulating running the guest program, the function should > > return the file name of the guest program (info->file_string), rather > > than the QEMU program itself, which we get from > > qemu_getauxval(AT_EXECFN). > > > > 2) Should it return the full path or as is? > > We tested the behavior of getauxval with a simple test program on > > Linux, and it turned out that it returned file path as is. For > > example, > > $ ./test > > getauxval(AT_EXECFN): ./test > > $ /usr/local/home/tmp/test > > getauxval(AT_EXECFN): /usr/local/home/tmp/test > > If you have compared the result with the real one it's fine for me. > > Resend tou patch without the AT_EXECFD part and it will be good. > > Thanks, > Laurent > Hi Laurent, Thank you so much for the review! :) Resent the v2 patch without AT_EXECFD: http://patchwork.ozlabs.org/patch/1247861/ Thanks, Lirong
diff --git a/linux-user/elfload.c b/linux-user/elfload.c index f3080a1635..7e0f3042f1 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1568,7 +1568,7 @@ struct exec ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1)) #define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1)) -#define DLINFO_ITEMS 15 +#define DLINFO_ITEMS 17 static inline void memcpy_fromfs(void * to, const void * from, unsigned long n) { @@ -1888,11 +1888,14 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s return sp; } -static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, +static abi_ulong create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, struct image_info *info, struct image_info *interp_info) { + abi_ulong p = bprm->p; + int argc = bprm->argc; + int envc = bprm->envc; abi_ulong sp; abi_ulong u_argc, u_argv, u_envp, u_auxv; int size; @@ -2032,6 +2035,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, NEW_AUX_ENT(AT_CLKTCK, (abi_ulong) sysconf(_SC_CLK_TCK)); NEW_AUX_ENT(AT_RANDOM, (abi_ulong) u_rand_bytes); NEW_AUX_ENT(AT_SECURE, (abi_ulong) qemu_getauxval(AT_SECURE)); + NEW_AUX_ENT(AT_EXECFN, info->file_string); + NEW_AUX_ENT(AT_EXECFD, bprm->fd); #ifdef ELF_HWCAP2 NEW_AUX_ENT(AT_HWCAP2, (abi_ulong) ELF_HWCAP2); @@ -2870,8 +2875,8 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) #endif } - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &elf_ex, - info, (elf_interpreter ? &interp_info : NULL)); + bprm->p = create_elf_tables(bprm, &elf_ex, info, + (elf_interpreter ? &interp_info : NULL)); info->start_stack = bprm->p; /* If we have an interpreter, set that as the program's entry point.
This change adds the support for AT_EXECFN and AT_EXECFD auxval. Signed-off-by: Lirong Yuan <yuanzi@google.com> --- linux-user/elfload.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)