linux-kernel - Re: [PATCH 7/7] exec: Implement kernel

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]

Message-ID: <202007150801.27B6690@keescook>
Date:   Wed, 15 Jul 2020 08:09:10 -0700
From:   Kees Cook <keescook@...omium.org>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     'Christoph Hellwig' <hch@...radead.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Al Viro <viro@...iv.linux.org.uk>,
        Luis Chamberlain <mcgrof@...nel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        James Morris <jmorris@...ei.org>,
        Kentaro Takeda <takedakn@...data.co.jp>,
        Casey Schaufler <casey@...aufler-ca.com>,
        John Johansen <john.johansen@...onical.com>
Subject: Re: [PATCH 7/7] exec: Implement kernel_execve

On Wed, Jul 15, 2020 at 02:55:50PM +0000, David Laight wrote:
> From: Christoph Hellwig
> > Sent: 15 July 2020 07:43
> > Subject: Re: [PATCH 7/7] exec: Implement kernel_execve
> > 
> > On Tue, Jul 14, 2020 at 02:49:23PM -0700, Kees Cook wrote:
> > > On Tue, Jul 14, 2020 at 08:31:40AM -0500, Eric W. Biederman wrote:
> > > > +static int count_strings_kernel(const char *const *argv)
> > > > +{
> > > > +	int i;
> > > > +
> > > > +	if (!argv)
> > > > +		return 0;
> > > > +
> > > > +	for (i = 0; argv[i]; ++i) {
> > > > +		if (i >= MAX_ARG_STRINGS)
> > > > +			return -E2BIG;
> > > > +		if (fatal_signal_pending(current))
> > > > +			return -ERESTARTNOHAND;
> > > > +		cond_resched();
> > > > +	}
> > > > +	return i;
> > > > +}
> > >
> > > I notice count() is only ever called with MAX_ARG_STRINGS. Perhaps
> > > refactor that too? (And maybe rename it to count_strings_user()?)
> 
> Thinks....
> If you setup env[] and argv[] on the new user stack early in exec processing
> then you may not need any limits at all - except the size of the user stack.
> Even the get_user() loop will hit an invalid address before the counter
> wraps (provided it is unsigned long).

*grumpy noises* Yes, but not in practice (if I'm understanding what you
mean). The expectations of a number of execution environments can be
really odd-ball. I've tried to collect the notes from over the years in
prepare_arg_pages()'s comments, and it mostly boils down to "there has
to be enough room for the exec to start" otherwise the exec ends up in a
hard-to-debug failure state (i.e. past the "point of no return", where you
get no useful information about the cause of the SEGV). So the point has
been to move as many of the setup checks as early as possible and report
about them if they fail. The argv processing is already very early, but
it needs to do the limit checks otherwise it'll just break after the exec
is underway and the process will just SEGV. (And ... some environments
will attempt to dynamically check the size of the argv space by growing
until it sees E2BIG, so we can't just remove it and let those hit SEGV.)

-- 
Kees Cook