[PATCH] [RFC] Allow recursion in binfmt_script and binfmt_misc

[PATCH] [RFC] Allow recursion in binfmt_script and binfmt_misc

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 3432 bytes --]

binfmt_script and binfmt_misc disallow recursion to avoid stack overflow
using sh_bang and misc_bang. It causes problem in some cases:

$ echo '#!/bin/ls' > /tmp/t0
$ echo '#!/tmp/t0' > /tmp/t1
$ echo '#!/tmp/t1' > /tmp/t2
$ chmod +x /tmp/t*
$ /tmp/t2
zsh: exec format error: /tmp/t2

Similar problem with binfmt_misc. 

I propose allow recursion, but limit its deep.

There is one more problem: on Alpha sh_bang used to remember if the 
application is TASO. I have no experience with Alpha. Is it possible to
use separate flag in struct linux_binprm for it?

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index f9c88d0..f95ae97 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -43,7 +43,7 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs)
 			return -ENOEXEC;
 	}
 
-	bprm->sh_bang = 1;	/* Well, the bang-shell is implicit... */
+	bprm->sh_bang++;	/* Well, the bang-shell is implicit... */
 	allow_write_access(bprm->file);
 	fput(bprm->file);
 	bprm->file = NULL;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 8d7e88e..50c665f 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -43,6 +43,8 @@ enum {Enabled, Magic};
 #define MISC_FMT_OPEN_BINARY (1<<30)
 #define MISC_FMT_CREDENTIALS (1<<29)
 
+#define MISC_FMT_MAX_RECURSION 4
+
 typedef struct {
 	struct list_head list;
 	unsigned long flags;		/* type, status, etc. */
@@ -117,7 +119,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 		goto _ret;
 
 	retval = -ENOEXEC;
-	if (bprm->misc_bang)
+	if (bprm->misc_bang >= MISC_FMT_MAX_RECURSION)
 		goto _ret;
 
 	/* to keep locking time low, we copy the interpreter string */
@@ -197,7 +199,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	if (retval < 0)
 		goto _error;
 
-	bprm->misc_bang = 1;
+	bprm->misc_bang++;
 
 	retval = search_binary_handler (bprm, regs);
 	if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 9e3963f..172c954 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -15,6 +15,8 @@
 #include <linux/err.h>
 #include <linux/fs.h>
 
+#define SCRIPT_FMT_MAX_RECURSION 4
+
 static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
 {
 	char *cp, *i_name, *i_arg;
@@ -22,14 +24,15 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
 	char interp[BINPRM_BUF_SIZE];
 	int retval;
 
-	if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') || (bprm->sh_bang)) 
+	if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
+	    (bprm->sh_bang >= SCRIPT_FMT_MAX_RECURSION))
 		return -ENOEXEC;
 	/*
 	 * This section does the #! interpretation.
 	 * Sorta complicated, but hopefully it will work.  -TYT
 	 */
 
-	bprm->sh_bang = 1;
+	bprm->sh_bang++;
 	allow_write_access(bprm->file);
 	fput(bprm->file);
 	bprm->file = NULL;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 826f623..1e21ff0 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -34,8 +34,8 @@ struct linux_binprm{
 #endif
 	struct mm_struct *mm;
 	unsigned long p; /* current top of mem */
-	unsigned int sh_bang:1,
-		     misc_bang:1;
+	unsigned char sh_bang;
+	unsigned char misc_bang;
 	struct file * file;
 	int e_uid, e_gid;
 	kernel_cap_t cap_post_exec_permitted;

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] Introduce field 'taso' into struct linux_binprm

[Kirill A. Shutemov]

This change is Alpha-specific. It adds field 'taso' into struct
linux_binprm to remember if the application is TASO. Previously,
field sh_bang was wsed for this purpose.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Pavel Emelyanov <xemul@openvz.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/alpha/include/asm/a.out.h |    2 +-
 fs/exec.c                      |    2 +-
 include/linux/binfmts.h        |    3 +++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/include/asm/a.out.h b/arch/alpha/include/asm/a.out.h
index 02ce847..acdc681 100644
--- a/arch/alpha/include/asm/a.out.h
+++ b/arch/alpha/include/asm/a.out.h
@@ -95,7 +95,7 @@ struct exec
    Worse, we have to notice the start address before swapping to use
    /sbin/loader, which of course is _not_ a TASO application.  */
 #define SET_AOUT_PERSONALITY(BFPM, EX) \
-	set_personality (((BFPM->sh_bang || EX.ah.entry < 0x100000000L \
+	set_personality (((BFPM->taso || EX.ah.entry < 0x100000000L \
 			   ? ADDR_LIMIT_32BIT : 0) | PER_OSF4))
 
 #endif /* __KERNEL__ */
diff --git a/fs/exec.c b/fs/exec.c
index 32993be..a46de17 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1189,7 +1189,7 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
 			return retval;
 
 		/* Remember if the application is TASO.  */
-		bprm->sh_bang = eh->ah.entry < 0x100000000UL;
+		bprm->taso = eh->ah.entry < 0x100000000UL;
 
 		bprm->file = file;
 		bprm->loader = loader;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 826f623..54980a3 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -36,6 +36,9 @@ struct linux_binprm{
 	unsigned long p; /* current top of mem */
 	unsigned int sh_bang:1,
 		     misc_bang:1;
+#ifdef __alpha__
+	unsigned int taso:1;
+#endif
 	struct file * file;
 	int e_uid, e_gid;
 	kernel_cap_t cap_post_exec_permitted;
-- 
1.5.6.5.GIT

[PATCH] Allow recursion in binfmt_script and binfmt_misc

[Kirill A. Shutemov]

binfmt_script and binfmt_misc disallow recursion to avoid stack overflow
using sh_bang and misc_bang. It causes problem in some cases:

$ echo '#!/bin/ls' > /tmp/t0
$ echo '#!/tmp/t0' > /tmp/t1
$ echo '#!/tmp/t1' > /tmp/t2
$ chmod +x /tmp/t*
$ /tmp/t2
zsh: exec format error: /tmp/t2

Similar problem with binfmt_misc.

This patch introduces field 'recursion_depth' into struct linux_binprm
to track recursion level in binfmt_misc and binfmt_script. If recursion
level more then BINPRM_MAX_RECURSION it generates -ENOEXEC.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Pavel Emelyanov <xemul@openvz.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 fs/binfmt_em86.c        |    2 +-
 fs/binfmt_misc.c        |    4 ++--
 fs/binfmt_script.c      |    5 +++--
 include/linux/binfmts.h |    4 ++--
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index f9c88d0..32fb00b 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -43,7 +43,7 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs)
 			return -ENOEXEC;
 	}
 
-	bprm->sh_bang = 1;	/* Well, the bang-shell is implicit... */
+	bprm->recursion_depth++; /* Well, the bang-shell is implicit... */
 	allow_write_access(bprm->file);
 	fput(bprm->file);
 	bprm->file = NULL;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 8d7e88e..f2744ab 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -117,7 +117,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 		goto _ret;
 
 	retval = -ENOEXEC;
-	if (bprm->misc_bang)
+	if (bprm->recursion_depth > BINPRM_MAX_RECURSION)
 		goto _ret;
 
 	/* to keep locking time low, we copy the interpreter string */
@@ -197,7 +197,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	if (retval < 0)
 		goto _error;
 
-	bprm->misc_bang = 1;
+	bprm->recursion_depth++;
 
 	retval = search_binary_handler (bprm, regs);
 	if (retval < 0)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 9e3963f..0834350 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -22,14 +22,15 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
 	char interp[BINPRM_BUF_SIZE];
 	int retval;
 
-	if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') || (bprm->sh_bang)) 
+	if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
+	    (bprm->recursion_depth > BINPRM_MAX_RECURSION))
 		return -ENOEXEC;
 	/*
 	 * This section does the #! interpretation.
 	 * Sorta complicated, but hopefully it will work.  -TYT
 	 */
 
-	bprm->sh_bang = 1;
+	bprm->recursion_depth++;
 	allow_write_access(bprm->file);
 	fput(bprm->file);
 	bprm->file = NULL;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 54980a3..1506b0d 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -34,8 +34,7 @@ struct linux_binprm{
 #endif
 	struct mm_struct *mm;
 	unsigned long p; /* current top of mem */
-	unsigned int sh_bang:1,
-		     misc_bang:1;
+	unsigned char recursion_depth;
 #ifdef __alpha__
 	unsigned int taso:1;
 #endif
@@ -61,6 +60,7 @@ struct linux_binprm{
 #define BINPRM_FLAGS_EXECFD_BIT 1
 #define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
 
+#define BINPRM_MAX_RECURSION 4
 
 /*
  * This structure defines the functions that are used to load the binary formats that
-- 
1.5.6.5.GIT

Re: [PATCH] Allow recursion in binfmt_script and binfmt_misc

[Andrew Morton]

On Sat,  6 Sep 2008 18:09:55 +0300
"Kirill A. Shutemov" <kirill@shutemov.name> wrote:

> binfmt_script and binfmt_misc disallow recursion to avoid stack overflow
> using sh_bang and misc_bang. It causes problem in some cases:
> 
> $ echo '#!/bin/ls' > /tmp/t0
> $ echo '#!/tmp/t0' > /tmp/t1
> $ echo '#!/tmp/t1' > /tmp/t2
> $ chmod +x /tmp/t*
> $ /tmp/t2
> zsh: exec format error: /tmp/t2
> 
> Similar problem with binfmt_misc.
> 
> This patch introduces field 'recursion_depth' into struct linux_binprm
> to track recursion level in binfmt_misc and binfmt_script. If recursion
> level more then BINPRM_MAX_RECURSION it generates -ENOEXEC.
> 
>
> ...
>
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -34,8 +34,7 @@ struct linux_binprm{
>  #endif
>  	struct mm_struct *mm;
>  	unsigned long p; /* current top of mem */
> -	unsigned int sh_bang:1,
> -		     misc_bang:1;
> +	unsigned char recursion_depth;
>  #ifdef __alpha__
>  	unsigned int taso:1;
>  #endif

That's a strange position in which to add the new field.  It will prevent
the compiler from using the same word for sh_bang, misc_bang and taso.

I fixed that up while fixing linux-next rejects.

> @@ -61,6 +60,7 @@ struct linux_binprm{
>  #define BINPRM_FLAGS_EXECFD_BIT 1
>  #define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
>  
> +#define BINPRM_MAX_RECURSION 4

Why "4"?

Why make linux_binprm.recursion_depth a u8?  There would be 
practically (or actually) zero cost to making it 32-bit.
Admittedly a depth >256 would be a bit odd, but did we gain 
anything from this restriction?

Re: [PATCH] Allow recursion in binfmt_script and binfmt_misc

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 2059 bytes --]

On Mon, Sep 08, 2008 at 03:39:21PM -0700, Andrew Morton wrote:
> On Sat,  6 Sep 2008 18:09:55 +0300
> "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> 
> > binfmt_script and binfmt_misc disallow recursion to avoid stack overflow
> > using sh_bang and misc_bang. It causes problem in some cases:
> > 
> > $ echo '#!/bin/ls' > /tmp/t0
> > $ echo '#!/tmp/t0' > /tmp/t1
> > $ echo '#!/tmp/t1' > /tmp/t2
> > $ chmod +x /tmp/t*
> > $ /tmp/t2
> > zsh: exec format error: /tmp/t2
> > 
> > Similar problem with binfmt_misc.
> > 
> > This patch introduces field 'recursion_depth' into struct linux_binprm
> > to track recursion level in binfmt_misc and binfmt_script. If recursion
> > level more then BINPRM_MAX_RECURSION it generates -ENOEXEC.
> > 
> >
> > ...
> >
> > --- a/include/linux/binfmts.h
> > +++ b/include/linux/binfmts.h
> > @@ -34,8 +34,7 @@ struct linux_binprm{
> >  #endif
> >  	struct mm_struct *mm;
> >  	unsigned long p; /* current top of mem */
> > -	unsigned int sh_bang:1,
> > -		     misc_bang:1;
> > +	unsigned char recursion_depth;
> >  #ifdef __alpha__
> >  	unsigned int taso:1;
> >  #endif
> 
> That's a strange position in which to add the new field.  It will prevent
> the compiler from using the same word for sh_bang, misc_bang and taso.
> 
> I fixed that up while fixing linux-next rejects.

Thanks.

> > @@ -61,6 +60,7 @@ struct linux_binprm{
> >  #define BINPRM_FLAGS_EXECFD_BIT 1
> >  #define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
> >  
> > +#define BINPRM_MAX_RECURSION 4
> 
> Why "4"?

It's enough for my goals. :)

We can increase it later if any user will need it.

> Why make linux_binprm.recursion_depth a u8?  There would be 
> practically (or actually) zero cost to making it 32-bit.
> Admittedly a depth >256 would be a bit odd, but did we gain 
> anything from this restriction?

No.

Should I repost patch with unsigned int recursion_depth?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]