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.=20
I propose allow recursion, but limit its deep.
There is one more problem: on Alpha sh_bang used to remember if the=20
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;
}
=20
- bprm->sh_bang =3D 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 =3D 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)
=20
+#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;
=20
retval =3D -ENOEXEC;
- if (bprm->misc_bang)
+ if (bprm->misc_bang >=3D MISC_FMT_MAX_RECURSION)
goto _ret;
=20
/* 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;
=20
- bprm->misc_bang =3D 1;
+ bprm->misc_bang++;
=20
retval =3D search_binary_handler (bprm, regs);
...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 ...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 = ...
On Sat, 6 Sep 2008 18:09:55 +0300 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. 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? --
It's enough for my goals. :) No. Should I repost patch with unsigned int recursion_depth? --=20 Regards, Kirill A. Shutemov + Belarus, Minsk + ALT Linux Team, http://www.altlinux.com/
