linux-hardening - Re: [PATCH 2/2] exec: Avoid pathological argc, envc, and bprm->p values

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] [day] [month] [year] [list]

Message-ID: <202406211253.7037F69@keescook>
Date: Fri, 21 Jun 2024 12:54:49 -0700
From: Kees Cook <kees@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Eric Biederman <ebiederm@...ssion.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 2/2] exec: Avoid pathological argc, envc, and bprm->p
 values

On Fri, Jun 21, 2024 at 06:21:15AM -0700, Guenter Roeck wrote:
> On 6/21/24 00:00, Kees Cook wrote:
> > On Thu, Jun 20, 2024 at 05:19:55PM -0700, Guenter Roeck wrote:
> > > Hi,
> > > 
> > > On Sun, May 19, 2024 at 07:16:12PM -0700, Kees Cook wrote:
> > > > Make sure nothing goes wrong with the string counters or the bprm's
> > > > belief about the stack pointer. Add checks and matching self-tests.
> > > > 
> > > > For 32-bit validation, this was run under 32-bit UML:
> > > > $ tools/testing/kunit/kunit.py run --make_options SUBARCH=i386 exec
> > > > 
> > > > Signed-off-by: Kees Cook <keescook@...omium.org>
> > > 
> > > With this patch in linux-next, the qemu m68k:mcf5208evb emulation
> > > fails to boot. The error is:
> > 
> > Eeek. Thanks for the report! I've dropped this patch from my for-next
> > tree.
> > 
> > > Run /init as init process
> > > Failed to execute /init (error -7)
> > 
> > -7 is E2BIG, so it's certainly one of the 3 new added checks. I must
> > have made a mistake in my reasoning about how bprm->p is initialized;
> > the other two checks seems extremely unlikely to be tripped.
> > 
> > I will try to get qemu set up and take a close look at what's happening.
> > While I'm doing that, if it's easy for you, can you try it with just
> > this removed (i.e. the other 2 new -E2BIG cases still in place):
> > 
> > 	/* Avoid a pathological bprm->p. */
> > 	if (bprm->p < limit)
> > 		return -E2BIG;
> 
> I added a printk:
> 
> argc: 1 envc: 2 p: 262140 limit: 2097152
>                 ^^^^^^^^^^^^^^^^^^^^^^^^
> Removing the check above does indeed fix the problem.

Thanks for checking this!

And I've found my mistake. "argmin" is only valid for CONFIG_MMU. And
you noticed this back in 2018. ;)

http://lkml.kernel.org/r/20181126122307.GA1660@redhat.com

I will try to fix this better so we don't trip over it again.

-- 
Kees Cook