linux-kernel - Re: [PATCH] Allow recursion in binfmt_script and binfmt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 8 Sep 2008 15:39:21 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Kirill A. Shutemov" <kirill@...temov.name>
Cc:	linux-kernel@...r.kernel.org, kirill@...temov.name,
	xemul@...nvz.org, viro@...iv.linux.org.uk
Subject: Re: [PATCH] Allow recursion in binfmt_script and binfmt_misc

On Sat,  6 Sep 2008 18:09:55 +0300
"Kirill A. Shutemov" <kirill@...temov.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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/