On Mon, Mar 2, 2020 at 10:31 AM Laurent Vivier <laur...@vivier.eu> wrote:
>
> Le 02/03/2020 à 19:18, Lirong Yuan a écrit :
> > On Mon, Mar 2, 2020 at 6:39 AM Laurent Vivier <laur...@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 <yua...@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
