linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
@ 2019-11-22 15:08 Laurent Vivier
  2019-12-03  9:40 ` Laurent Vivier
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Laurent Vivier @ 2019-11-22 15:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fsdevel, Alexander Viro, James Bottomley, Laurent Vivier

It can be useful to the interpreter to know which flags are in use.

For instance, knowing if the preserve-argv[0] is in use would
allow to skip the pathname argument.

This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
content of the binfmt flags (special flags: P, F, C, O).

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---

Notes:
    v2: only pass special flags (remove Magic and Enabled flags)

 fs/binfmt_elf.c         | 2 +-
 fs/binfmt_elf_fdpic.c   | 2 +-
 fs/binfmt_misc.c        | 6 ++++++
 include/linux/binfmts.h | 2 +-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index c5642bcb6b46..7a34c03e5857 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
+	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
 	NEW_AUX_ENT(AT_ENTRY, exec->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 d86ebd0dcc3d..8fe839be125e 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -647,7 +647,7 @@ 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);
+	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -48,6 +48,9 @@ enum {Enabled, Magic};
 #define MISC_FMT_OPEN_BINARY (1 << 30)
 #define MISC_FMT_CREDENTIALS (1 << 29)
 #define MISC_FMT_OPEN_FILE (1 << 28)
+#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
+			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
+
 
 typedef struct {
 	struct list_head list;
@@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	if (!fmt)
 		return retval;
 
+	/* pass special flags to the interpreter */
+	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
+
 	/* Need to be able to load the file after exec */
 	retval = -ENOENT;
 	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index b40fc633f3be..dae0d0d7b84d 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -60,7 +60,7 @@ struct linux_binprm {
 				   different for binfmt_{misc,script} */
 	unsigned interp_flags;
 	unsigned interp_data;
-	unsigned long loader, exec;
+	unsigned long loader, exec, fmt_flags;
 
 	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2019-11-22 15:08 [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter Laurent Vivier
@ 2019-12-03  9:40 ` Laurent Vivier
  2019-12-16  9:22 ` Laurent Vivier
  2020-01-07 14:50 ` Laurent Vivier
  2 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2019-12-03  9:40 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, Alexander Viro, James Bottomley

Ping

Thanks,
Laurent


Le 22/11/2019 à 16:08, Laurent Vivier a écrit :
> It can be useful to the interpreter to know which flags are in use.
> 
> For instance, knowing if the preserve-argv[0] is in use would
> allow to skip the pathname argument.
> 
> This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
> content of the binfmt flags (special flags: P, F, C, O).
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 
> Notes:
>     v2: only pass special flags (remove Magic and Enabled flags)
> 
>  fs/binfmt_elf.c         | 2 +-
>  fs/binfmt_elf_fdpic.c   | 2 +-
>  fs/binfmt_misc.c        | 6 ++++++
>  include/linux/binfmts.h | 2 +-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index c5642bcb6b46..7a34c03e5857 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
> +	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
>  	NEW_AUX_ENT(AT_ENTRY, exec->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 d86ebd0dcc3d..8fe839be125e 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -647,7 +647,7 @@ 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);
> +	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -48,6 +48,9 @@ enum {Enabled, Magic};
>  #define MISC_FMT_OPEN_BINARY (1 << 30)
>  #define MISC_FMT_CREDENTIALS (1 << 29)
>  #define MISC_FMT_OPEN_FILE (1 << 28)
> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
> +
>  
>  typedef struct {
>  	struct list_head list;
> @@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
>  	if (!fmt)
>  		return retval;
>  
> +	/* pass special flags to the interpreter */
> +	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
> +
>  	/* Need to be able to load the file after exec */
>  	retval = -ENOENT;
>  	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index b40fc633f3be..dae0d0d7b84d 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -60,7 +60,7 @@ struct linux_binprm {
>  				   different for binfmt_{misc,script} */
>  	unsigned interp_flags;
>  	unsigned interp_data;
> -	unsigned long loader, exec;
> +	unsigned long loader, exec, fmt_flags;
>  
>  	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
>  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2019-11-22 15:08 [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter Laurent Vivier
  2019-12-03  9:40 ` Laurent Vivier
@ 2019-12-16  9:22 ` Laurent Vivier
  2019-12-26 14:08   ` [EXTERNAL]Re: " Yunqiang Su
  2020-01-07 14:50 ` Laurent Vivier
  2 siblings, 1 reply; 8+ messages in thread
From: Laurent Vivier @ 2019-12-16  9:22 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, Alexander Viro, James Bottomley, YunQiang Su

Hi,

this feature is really needed and requested by several binfmt_misc
users. It is difficult to manage this at user level without this
information.

I've found that someone else tried to address this problem in the past
in the same way:
https://patchwork.kernel.org/patch/10902935/

Can a maintainer/reviewer have a look at this and agrees or disagrees?

I will update getauxval(3) manual page with the new flags in the case
one of these patches is merged.

Thanks,
Laurent


Le 22/11/2019 à 16:08, Laurent Vivier a écrit :
> It can be useful to the interpreter to know which flags are in use.
> 
> For instance, knowing if the preserve-argv[0] is in use would
> allow to skip the pathname argument.
> 
> This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
> content of the binfmt flags (special flags: P, F, C, O).
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 
> Notes:
>     v2: only pass special flags (remove Magic and Enabled flags)
> 
>  fs/binfmt_elf.c         | 2 +-
>  fs/binfmt_elf_fdpic.c   | 2 +-
>  fs/binfmt_misc.c        | 6 ++++++
>  include/linux/binfmts.h | 2 +-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index c5642bcb6b46..7a34c03e5857 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
> +	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
>  	NEW_AUX_ENT(AT_ENTRY, exec->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 d86ebd0dcc3d..8fe839be125e 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -647,7 +647,7 @@ 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);
> +	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -48,6 +48,9 @@ enum {Enabled, Magic};
>  #define MISC_FMT_OPEN_BINARY (1 << 30)
>  #define MISC_FMT_CREDENTIALS (1 << 29)
>  #define MISC_FMT_OPEN_FILE (1 << 28)
> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
> +
>  
>  typedef struct {
>  	struct list_head list;
> @@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
>  	if (!fmt)
>  		return retval;
>  
> +	/* pass special flags to the interpreter */
> +	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
> +
>  	/* Need to be able to load the file after exec */
>  	retval = -ENOENT;
>  	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index b40fc633f3be..dae0d0d7b84d 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -60,7 +60,7 @@ struct linux_binprm {
>  				   different for binfmt_{misc,script} */
>  	unsigned interp_flags;
>  	unsigned interp_data;
> -	unsigned long loader, exec;
> +	unsigned long loader, exec, fmt_flags;
>  
>  	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
>  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXTERNAL]Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2019-12-16  9:22 ` Laurent Vivier
@ 2019-12-26 14:08   ` Yunqiang Su
  0 siblings, 0 replies; 8+ messages in thread
From: Yunqiang Su @ 2019-12-26 14:08 UTC (permalink / raw)
  To: Laurent Vivier, linux-kernel
  Cc: linux-fsdevel, Alexander Viro, James Bottomley

Ping...

We really need it. since qemu-user mode have no other choice to get the info about whether P flag is set.

> -----Original Message-----
> From: Laurent Vivier <laurent@vivier.eu>
> Sent: 2019年12月16日 17:22
> To: linux-kernel@vger.kernel.org
> Cc: linux-fsdevel@vger.kernel.org; Alexander Viro <viro@zeniv.linux.org.uk>;
> James Bottomley <James.Bottomley@HansenPartnership.com>; Yunqiang Su
> <ysu@wavecomp.com>
> Subject: [EXTERNAL]Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the
> interpreter
> 
> Hi,
> 
> this feature is really needed and requested by several binfmt_misc users. It is
> difficult to manage this at user level without this information.
> 
> I've found that someone else tried to address this problem in the past in the
> same way:
> https://patchwork.kernel.org/patch/10902935/
> 
> Can a maintainer/reviewer have a look at this and agrees or disagrees?
> 
> I will update getauxval(3) manual page with the new flags in the case one of
> these patches is merged.
> 
> Thanks,
> Laurent
> 
> 
> Le 22/11/2019 à 16:08, Laurent Vivier a écrit :
> > It can be useful to the interpreter to know which flags are in use.
> >
> > For instance, knowing if the preserve-argv[0] is in use would allow to
> > skip the pathname argument.
> >
> > This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
> > content of the binfmt flags (special flags: P, F, C, O).
> >
> > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> > ---
> >
> > Notes:
> >     v2: only pass special flags (remove Magic and Enabled flags)
> >
> >  fs/binfmt_elf.c         | 2 +-
> >  fs/binfmt_elf_fdpic.c   | 2 +-
> >  fs/binfmt_misc.c        | 6 ++++++
> >  include/linux/binfmts.h | 2 +-
> >  4 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index
> > c5642bcb6b46..7a34c03e5857 100644
> > --- a/fs/binfmt_elf.c
> > +++ b/fs/binfmt_elf.c
> > @@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
> > +	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
> >  	NEW_AUX_ENT(AT_ENTRY, exec->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
> > d86ebd0dcc3d..8fe839be125e 100644
> > --- a/fs/binfmt_elf_fdpic.c
> > +++ b/fs/binfmt_elf_fdpic.c
> > @@ -647,7 +647,7 @@ 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);
> > +	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
> > --- a/fs/binfmt_misc.c
> > +++ b/fs/binfmt_misc.c
> > @@ -48,6 +48,9 @@ enum {Enabled, Magic};  #define
> MISC_FMT_OPEN_BINARY
> > (1 << 30)  #define MISC_FMT_CREDENTIALS (1 << 29)  #define
> > MISC_FMT_OPEN_FILE (1 << 28)
> > +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 |
> MISC_FMT_OPEN_BINARY | \
> > +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
> > +
> >
> >  typedef struct {
> >  	struct list_head list;
> > @@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm
> *bprm)
> >  	if (!fmt)
> >  		return retval;
> >
> > +	/* pass special flags to the interpreter */
> > +	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
> > +
> >  	/* Need to be able to load the file after exec */
> >  	retval = -ENOENT;
> >  	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) diff --git
> > a/include/linux/binfmts.h b/include/linux/binfmts.h index
> > b40fc633f3be..dae0d0d7b84d 100644
> > --- a/include/linux/binfmts.h
> > +++ b/include/linux/binfmts.h
> > @@ -60,7 +60,7 @@ struct linux_binprm {
> >  				   different for binfmt_{misc,script} */
> >  	unsigned interp_flags;
> >  	unsigned interp_data;
> > -	unsigned long loader, exec;
> > +	unsigned long loader, exec, fmt_flags;
> >
> >  	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
> >
> >


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2019-11-22 15:08 [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter Laurent Vivier
  2019-12-03  9:40 ` Laurent Vivier
  2019-12-16  9:22 ` Laurent Vivier
@ 2020-01-07 14:50 ` Laurent Vivier
  2020-01-15 12:19   ` Laurent Vivier
  2 siblings, 1 reply; 8+ messages in thread
From: Laurent Vivier @ 2020-01-07 14:50 UTC (permalink / raw)
  To: linux-kernel, Alexander Viro; +Cc: linux-fsdevel, James Bottomley

Hi,

this change is simple, easy to read and understand but it is really
needed by user space application interpreter to know the status of the
system configuration.

Could we have a comment saying if there is a problem or if it is good to
be merged?

Thanks,
Laurent

Le 22/11/2019 à 16:08, Laurent Vivier a écrit :
> It can be useful to the interpreter to know which flags are in use.
> 
> For instance, knowing if the preserve-argv[0] is in use would
> allow to skip the pathname argument.
> 
> This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
> content of the binfmt flags (special flags: P, F, C, O).
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 
> Notes:
>     v2: only pass special flags (remove Magic and Enabled flags)
> 
>  fs/binfmt_elf.c         | 2 +-
>  fs/binfmt_elf_fdpic.c   | 2 +-
>  fs/binfmt_misc.c        | 6 ++++++
>  include/linux/binfmts.h | 2 +-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index c5642bcb6b46..7a34c03e5857 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
> +	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
>  	NEW_AUX_ENT(AT_ENTRY, exec->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 d86ebd0dcc3d..8fe839be125e 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -647,7 +647,7 @@ 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);
> +	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -48,6 +48,9 @@ enum {Enabled, Magic};
>  #define MISC_FMT_OPEN_BINARY (1 << 30)
>  #define MISC_FMT_CREDENTIALS (1 << 29)
>  #define MISC_FMT_OPEN_FILE (1 << 28)
> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
> +
>  
>  typedef struct {
>  	struct list_head list;
> @@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
>  	if (!fmt)
>  		return retval;
>  
> +	/* pass special flags to the interpreter */
> +	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
> +
>  	/* Need to be able to load the file after exec */
>  	retval = -ENOENT;
>  	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index b40fc633f3be..dae0d0d7b84d 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -60,7 +60,7 @@ struct linux_binprm {
>  				   different for binfmt_{misc,script} */
>  	unsigned interp_flags;
>  	unsigned interp_data;
> -	unsigned long loader, exec;
> +	unsigned long loader, exec, fmt_flags;
>  
>  	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
>  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2020-01-07 14:50 ` Laurent Vivier
@ 2020-01-15 12:19   ` Laurent Vivier
  2020-01-15 13:55     ` Al Viro
  0 siblings, 1 reply; 8+ messages in thread
From: Laurent Vivier @ 2020-01-15 12:19 UTC (permalink / raw)
  To: linux-kernel, Alexander Viro; +Cc: linux-fsdevel, James Bottomley

Le 07/01/2020 à 15:50, Laurent Vivier a écrit :
> Hi,
> 
> this change is simple, easy to read and understand but it is really
> needed by user space application interpreter to know the status of the
> system configuration.
> 
> Could we have a comment saying if there is a problem or if it is good to
> be merged?

Anyone?

> Thanks,
> Laurent
> 
> Le 22/11/2019 à 16:08, Laurent Vivier a écrit :
>> It can be useful to the interpreter to know which flags are in use.
>>
>> For instance, knowing if the preserve-argv[0] is in use would
>> allow to skip the pathname argument.
>>
>> This patch uses an unused auxiliary vector, AT_FLAGS,  to pass the
>> content of the binfmt flags (special flags: P, F, C, O).
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>
>> Notes:
>>     v2: only pass special flags (remove Magic and Enabled flags)
>>
>>  fs/binfmt_elf.c         | 2 +-
>>  fs/binfmt_elf_fdpic.c   | 2 +-
>>  fs/binfmt_misc.c        | 6 ++++++
>>  include/linux/binfmts.h | 2 +-
>>  4 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
>> index c5642bcb6b46..7a34c03e5857 100644
>> --- a/fs/binfmt_elf.c
>> +++ b/fs/binfmt_elf.c
>> @@ -250,7 +250,7 @@ create_elf_tables(struct linux_binprm *bprm, 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);
>> +	NEW_AUX_ENT(AT_FLAGS, bprm->fmt_flags);
>>  	NEW_AUX_ENT(AT_ENTRY, exec->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 d86ebd0dcc3d..8fe839be125e 100644
>> --- a/fs/binfmt_elf_fdpic.c
>> +++ b/fs/binfmt_elf_fdpic.c
>> @@ -647,7 +647,7 @@ 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);
>> +	NEW_AUX_ENT(AT_FLAGS,	bprm->fmt_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..25a392f23409 100644
>> --- a/fs/binfmt_misc.c
>> +++ b/fs/binfmt_misc.c
>> @@ -48,6 +48,9 @@ enum {Enabled, Magic};
>>  #define MISC_FMT_OPEN_BINARY (1 << 30)
>>  #define MISC_FMT_CREDENTIALS (1 << 29)
>>  #define MISC_FMT_OPEN_FILE (1 << 28)
>> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
>> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
>> +
>>  
>>  typedef struct {
>>  	struct list_head list;
>> @@ -149,6 +152,9 @@ static int load_misc_binary(struct linux_binprm *bprm)
>>  	if (!fmt)
>>  		return retval;
>>  
>> +	/* pass special flags to the interpreter */
>> +	bprm->fmt_flags = fmt->flags & MISC_FMT_FLAGS_MASK;
>> +
>>  	/* Need to be able to load the file after exec */
>>  	retval = -ENOENT;
>>  	if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE)
>> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
>> index b40fc633f3be..dae0d0d7b84d 100644
>> --- a/include/linux/binfmts.h
>> +++ b/include/linux/binfmts.h
>> @@ -60,7 +60,7 @@ struct linux_binprm {
>>  				   different for binfmt_{misc,script} */
>>  	unsigned interp_flags;
>>  	unsigned interp_data;
>> -	unsigned long loader, exec;
>> +	unsigned long loader, exec, fmt_flags;
>>  
>>  	struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */
>>  
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2020-01-15 12:19   ` Laurent Vivier
@ 2020-01-15 13:55     ` Al Viro
  2020-01-15 14:07       ` Laurent Vivier
  0 siblings, 1 reply; 8+ messages in thread
From: Al Viro @ 2020-01-15 13:55 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: linux-kernel, linux-fsdevel, James Bottomley

On Wed, Jan 15, 2020 at 01:19:16PM +0100, Laurent Vivier wrote:
> Le 07/01/2020 à 15:50, Laurent Vivier a écrit :
> > Hi,
> > 
> > this change is simple, easy to read and understand but it is really
> > needed by user space application interpreter to know the status of the
> > system configuration.
> > 
> > Could we have a comment saying if there is a problem or if it is good to
> > be merged?
> 
> Anyone?

	FWIW, one thing that looks worrying here is that these bits become
userland ABI after this patch - specific values passed in that thing
can't be changed.  And no a single mention of that in fs/binfmt_misc.c,
leaving a nasty trap.  As far as one can tell, their values are fair game
for reordering, etc. - not even visible outside of fs/binfmt_misc.c;
purely internal constants.  And the effect of such modifications after
your patch will not be "everything breaks, patch gets caught by somebody's
tests" - it will be a quiet breakage for some users.

> >>  #define MISC_FMT_OPEN_BINARY (1 << 30)
> >>  #define MISC_FMT_CREDENTIALS (1 << 29)
> >>  #define MISC_FMT_OPEN_FILE (1 << 28)
> >> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
> >> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)

IOW, you are making those parts of userland ABI cast in stone forever.
Whether this bit assignment does make sense or not, such things really
should not be hidden.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter
  2020-01-15 13:55     ` Al Viro
@ 2020-01-15 14:07       ` Laurent Vivier
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2020-01-15 14:07 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, linux-fsdevel, James Bottomley, YunQiang Su

Le 15/01/2020 à 14:55, Al Viro a écrit :
> On Wed, Jan 15, 2020 at 01:19:16PM +0100, Laurent Vivier wrote:
>> Le 07/01/2020 à 15:50, Laurent Vivier a écrit :
>>> Hi,
>>>
>>> this change is simple, easy to read and understand but it is really
>>> needed by user space application interpreter to know the status of the
>>> system configuration.
>>>
>>> Could we have a comment saying if there is a problem or if it is good to
>>> be merged?
>>
>> Anyone?
> 
> 	FWIW, one thing that looks worrying here is that these bits become
> userland ABI after this patch - specific values passed in that thing
> can't be changed.  And no a single mention of that in fs/binfmt_misc.c,
> leaving a nasty trap.  As far as one can tell, their values are fair game
> for reordering, etc. - not even visible outside of fs/binfmt_misc.c;
> purely internal constants.  And the effect of such modifications after
> your patch will not be "everything breaks, patch gets caught by somebody's
> tests" - it will be a quiet breakage for some users.
> 
>>>>  #define MISC_FMT_OPEN_BINARY (1 << 30)
>>>>  #define MISC_FMT_CREDENTIALS (1 << 29)
>>>>  #define MISC_FMT_OPEN_FILE (1 << 28)
>>>> +#define MISC_FMT_FLAGS_MASK (MISC_FMT_PRESERVE_ARGV0 | MISC_FMT_OPEN_BINARY | \
>>>> +			     MISC_FMT_CREDENTIALS | MISC_FMT_OPEN_FILE)
> 
> IOW, you are making those parts of userland ABI cast in stone forever.
> Whether this bit assignment does make sense or not, such things really
> should not be hidden.
> 

Thank you for your answer.

So I think the patch from YunQiang Su is a better approach than mine,
much cleaner, see:

  binfmt_misc: pass info about P flag by AT_FLAGS
  https://patchwork.kernel.org/patch/10902935/

It does the same thing as my patch but uses a dedicated value for AT_FLAGS.

Perhaps YunQiang can send a new version (without the kdebug() part)?

Thanks,
Laurent

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-15 14:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 15:08 [RFC v2] binfmt_misc: pass binfmt_misc flags to the interpreter Laurent Vivier
2019-12-03  9:40 ` Laurent Vivier
2019-12-16  9:22 ` Laurent Vivier
2019-12-26 14:08   ` [EXTERNAL]Re: " Yunqiang Su
2020-01-07 14:50 ` Laurent Vivier
2020-01-15 12:19   ` Laurent Vivier
2020-01-15 13:55     ` Al Viro
2020-01-15 14:07       ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).