Message ID | 20200306080905.173466-1-syq@debian.org |
---|---|
State | New |
Headers | show |
Series | binfmt_misc: pass binfmt_misc P flag to the interpreter | expand |
* YunQiang Su: > + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) > + flags |= AT_FLAGS_PRESERVE_ARGV0; > + NEW_AUX_ENT(AT_FLAGS, flags); Is it necessary to reuse AT_FLAGS? I think it's cleaner to define a separate AT_ tag dedicated to binfmt_misc.
Le 06/03/2020 à 09:13, Florian Weimer a écrit : > * YunQiang Su: > >> + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) >> + flags |= AT_FLAGS_PRESERVE_ARGV0; >> + NEW_AUX_ENT(AT_FLAGS, flags); > > Is it necessary to reuse AT_FLAGS? I think it's cleaner to define a > separate AT_ tag dedicated to binfmt_misc. > Not necessary, but it seemed simpler and cleaner to re-use a flag that is marked as unused and with a name matching the new role. It avoids to patch other packages (like glibc) to add it as it is already defined. Thanks, Laurent
* Laurent Vivier: > Le 06/03/2020 à 09:13, Florian Weimer a écrit : >> * YunQiang Su: >> >>> + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) >>> + flags |= AT_FLAGS_PRESERVE_ARGV0; >>> + NEW_AUX_ENT(AT_FLAGS, flags); >> >> Is it necessary to reuse AT_FLAGS? I think it's cleaner to define a >> separate AT_ tag dedicated to binfmt_misc. > > Not necessary, but it seemed simpler and cleaner to re-use a flag that > is marked as unused and with a name matching the new role. It avoids to > patch other packages (like glibc) to add it as it is already defined. You still need to define AT_FLAGS_PRESERVE_ARGV0. At that point, you might as well define AT_BINFMT and AT_BINFMT_PRESERVE_ARGV0.
Le 06/03/2020 à 09:37, Florian Weimer a écrit : > * Laurent Vivier: > >> Le 06/03/2020 à 09:13, Florian Weimer a écrit : >>> * YunQiang Su: >>> >>>> + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) >>>> + flags |= AT_FLAGS_PRESERVE_ARGV0; >>>> + NEW_AUX_ENT(AT_FLAGS, flags); >>> >>> Is it necessary to reuse AT_FLAGS? I think it's cleaner to define a >>> separate AT_ tag dedicated to binfmt_misc. >> >> Not necessary, but it seemed simpler and cleaner to re-use a flag that >> is marked as unused and with a name matching the new role. It avoids to >> patch other packages (like glibc) to add it as it is already defined. > > You still need to define AT_FLAGS_PRESERVE_ARGV0. At that point, you > might as well define AT_BINFMT and AT_BINFMT_PRESERVE_ARGV0. > Yes, you're right. But is there any reason to not reuse AT_FLAGS? Thanks, Laurent
Laurent Vivier <laurent@vivier.eu> 于2020年3月6日周五 下午7:13写道: > > Le 06/03/2020 à 09:37, Florian Weimer a écrit : > > * Laurent Vivier: > > > >> Le 06/03/2020 à 09:13, Florian Weimer a écrit : > >>> * YunQiang Su: > >>> > >>>> + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) > >>>> + flags |= AT_FLAGS_PRESERVE_ARGV0; > >>>> + NEW_AUX_ENT(AT_FLAGS, flags); > >>> > >>> Is it necessary to reuse AT_FLAGS? I think it's cleaner to define a > >>> separate AT_ tag dedicated to binfmt_misc. > >> > >> Not necessary, but it seemed simpler and cleaner to re-use a flag that > >> is marked as unused and with a name matching the new role. It avoids to > >> patch other packages (like glibc) to add it as it is already defined. > > > > You still need to define AT_FLAGS_PRESERVE_ARGV0. At that point, you > > might as well define AT_BINFMT and AT_BINFMT_PRESERVE_ARGV0. > > > > Yes, you're right. > > But is there any reason to not reuse AT_FLAGS? AT_* only has 32 slot and now. I was afraid that maybe we shouldn't take one. /* AT_* values 18 through 22 are reserved */ 27,28,29,30 are not used now. Which should we use? > > Thanks, > Laurent
* YunQiang Su: > AT_* only has 32 slot and now. I was afraid that maybe we shouldn't take one. > /* AT_* values 18 through 22 are reserved */ > 27,28,29,30 are not used now. > Which should we use? Where does this limit of 32 tags come from? I don't see it from a userspace perspective.
Florian Weimer <fw@deneb.enyo.de> 于2020年3月6日周五 下午7:42写道: > > * YunQiang Su: > > > AT_* only has 32 slot and now. I was afraid that maybe we shouldn't take one. > > /* AT_* values 18 through 22 are reserved */ > > 27,28,29,30 are not used now. > > Which should we use? > > Where does this limit of 32 tags come from? I don't see it from a > userspace perspective. Sorry it is my mistake: In linux/auxvec.h, I saw #define AT_RANDOM 25 /* address of 16 random bytes */ #define AT_HWCAP2 26 /* extension of AT_HWCAP */ #define AT_EXECFN 31 /* filename of program */ The number jump to 31 from 26. It is my fault: in x86_64-linux-gnu/bits/auxv.h, the max number is 47 now.
* YunQiang Su: > Florian Weimer <fw@deneb.enyo.de> 于2020年3月6日周五 下午7:42写道: >> >> * YunQiang Su: >> >> > AT_* only has 32 slot and now. I was afraid that maybe we shouldn't take one. >> > /* AT_* values 18 through 22 are reserved */ >> > 27,28,29,30 are not used now. >> > Which should we use? >> >> Where does this limit of 32 tags come from? I don't see it from a >> userspace perspective. > > Sorry it is my mistake: In linux/auxvec.h, I saw > > #define AT_RANDOM 25 /* address of 16 random bytes */ > #define AT_HWCAP2 26 /* extension of AT_HWCAP */ > > #define AT_EXECFN 31 /* filename of program */ > > The number jump to 31 from 26. > > It is my fault: in x86_64-linux-gnu/bits/auxv.h, the max number is 47 now. So AT_* tags aren't a scarce resource after all?
Le 06/03/2020 à 12:48, YunQiang Su a écrit : > Florian Weimer <fw@deneb.enyo.de> 于2020年3月6日周五 下午7:42写道: >> >> * YunQiang Su: >> >>> AT_* only has 32 slot and now. I was afraid that maybe we shouldn't take one. >>> /* AT_* values 18 through 22 are reserved */ >>> 27,28,29,30 are not used now. >>> Which should we use? >> >> Where does this limit of 32 tags come from? I don't see it from a >> userspace perspective. > > Sorry it is my mistake: In linux/auxvec.h, I saw > > #define AT_RANDOM 25 /* address of 16 random bytes */ > #define AT_HWCAP2 26 /* extension of AT_HWCAP */ > > #define AT_EXECFN 31 /* filename of program */ > > The number jump to 31 from 26. > > It is my fault: in x86_64-linux-gnu/bits/auxv.h, the max number is 47 now. > Numbers starting from 32 are arch specific. 18 to 22 are also reserved (and used by some archs). (linux/arch/*/include/uapi/asm/auxvec.h) So there is only 4 entries available (27 to 30) (linux/include/uapi/linux/auxvec.h) Do we want to waste one more entry whereas we can use an unused one? Thanks, Laurent
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 13f25e241ac4..e9f5d135b847 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -178,7 +178,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, const char *k_base_platform = ELF_BASE_PLATFORM; unsigned char k_rand_bytes[16]; int items; - elf_addr_t *elf_info; + elf_addr_t *elf_info, flags = 0; int ei_index; const struct cred *cred = current_cred(); struct vm_area_struct *vma; @@ -253,7 +253,9 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr)); NEW_AUX_ENT(AT_PHNUM, exec->e_phnum); NEW_AUX_ENT(AT_BASE, interp_load_addr); - NEW_AUX_ENT(AT_FLAGS, 0); + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) + flags |= AT_FLAGS_PRESERVE_ARGV0; + NEW_AUX_ENT(AT_FLAGS, flags); NEW_AUX_ENT(AT_ENTRY, e_entry); NEW_AUX_ENT(AT_UID, from_kuid_munged(cred->user_ns, cred->uid)); NEW_AUX_ENT(AT_EUID, from_kuid_munged(cred->user_ns, cred->euid)); diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 240f66663543..abb90d82aa58 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -507,6 +507,7 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm, char __user *u_platform, *u_base_platform, *p; int loop; int nr; /* reset for each csp adjustment */ + unsigned long flags = 0; #ifdef CONFIG_MMU /* In some cases (e.g. Hyper-Threading), we want to avoid L1 evictions @@ -647,7 +648,9 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm, NEW_AUX_ENT(AT_PHENT, sizeof(struct elf_phdr)); NEW_AUX_ENT(AT_PHNUM, exec_params->hdr.e_phnum); NEW_AUX_ENT(AT_BASE, interp_params->elfhdr_addr); - NEW_AUX_ENT(AT_FLAGS, 0); + if (bprm->interp_flags & BINPRM_FLAGS_PRESERVE_ARGV0) + flags |= AT_FLAGS_PRESERVE_ARGV0; + NEW_AUX_ENT(AT_FLAGS, flags); NEW_AUX_ENT(AT_ENTRY, exec_params->entry_addr); NEW_AUX_ENT(AT_UID, (elf_addr_t) from_kuid_munged(cred->user_ns, cred->uid)); NEW_AUX_ENT(AT_EUID, (elf_addr_t) from_kuid_munged(cred->user_ns, cred->euid)); diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index cdb45829354d..b9acdd26a654 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -154,7 +154,9 @@ static int load_misc_binary(struct linux_binprm *bprm) if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) goto ret; - if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) { + if (fmt->flags & MISC_FMT_PRESERVE_ARGV0) { + bprm->interp_flags |= BINPRM_FLAGS_PRESERVE_ARGV0; + } else { retval = remove_arg_zero(bprm); if (retval) goto ret; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index b40fc633f3be..265b80d5fd6f 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -78,6 +78,10 @@ struct linux_binprm { #define BINPRM_FLAGS_PATH_INACCESSIBLE_BIT 2 #define BINPRM_FLAGS_PATH_INACCESSIBLE (1 << BINPRM_FLAGS_PATH_INACCESSIBLE_BIT) +/* if preserve the argv0 for the interpreter */ +#define BINPRM_FLAGS_PRESERVE_ARGV0_BIT 3 +#define BINPRM_FLAGS_PRESERVE_ARGV0 (1 << BINPRM_FLAGS_PRESERVE_ARGV0_BIT) + /* Function parameter for binfmt->coredump */ struct coredump_params { const kernel_siginfo_t *siginfo; diff --git a/include/uapi/linux/binfmts.h b/include/uapi/linux/binfmts.h index 689025d9c185..a70747416130 100644 --- a/include/uapi/linux/binfmts.h +++ b/include/uapi/linux/binfmts.h @@ -18,4 +18,8 @@ struct pt_regs; /* sizeof(linux_binprm->buf) */ #define BINPRM_BUF_SIZE 256 +/* if preserve the argv0 for the interpreter */ +#define AT_FLAGS_PRESERVE_ARGV0_BIT 0 +#define AT_FLAGS_PRESERVE_ARGV0 (1 << AT_FLAGS_PRESERVE_ARGV0_BIT) + #endif /* _UAPI_LINUX_BINFMTS_H */