linux-kernel - Re: [PATCH RFC 3/8] openat2: explicitly return -E2BIG for (usize > PAGE

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240902.160305-cuddly.doc.quaint.provider-RsRaXpw78cll@cyphar.com>
Date: Tue, 3 Sep 2024 02:08:26 +1000
From: Aleksa Sarai <cyphar@...har.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, 
	Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>, 
	Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>, 
	Benjamin Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, 
	Valentin Schneider <vschneid@...hat.com>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, shuah <shuah@...nel.org>, 
	Kees Cook <kees@...nel.org>, Florian Weimer <fweimer@...hat.com>, 
	Mark Rutland <mark.rutland@....com>, linux-kernel@...r.kernel.org, linux-api@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, Linux-Arch <linux-arch@...r.kernel.org>, 
	linux-kselftest@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH RFC 3/8] openat2: explicitly return -E2BIG for (usize >
 PAGE_SIZE)

On 2024-09-02, Arnd Bergmann <arnd@...db.de> wrote:
> On Mon, Sep 2, 2024, at 07:06, Aleksa Sarai wrote:
> > While we do currently return -EFAULT in this case, it seems prudent to
> > follow the behaviour of other syscalls like clone3. It seems quite
> > unlikely that anyone depends on this error code being EFAULT, but we can
> > always revert this if it turns out to be an issue.
> 
> Right, it's probably a good idea to have a limit there rather than
> having a busy loop with a user-provided length when the only bound is
> the available virtual memory.
> 
> >  	if (unlikely(usize < OPEN_HOW_SIZE_VER0))
> >  		return -EINVAL;
> > +	if (unlikely(usize > PAGE_SIZE))
> > +		return -E2BIG;
> > 
> 
> Is PAGE_SIZE significant here? If there is a need to enforce a limit,
> I would expect this to be the same regardless of kernel configuration,
> since the structure layout is also independent of the configuration.

PAGE_SIZE is what clone3, perf_event_open, sched_setattr, bpf, etc all
use. The idea was that PAGE_SIZE is the absolute limit of any reasonable
extensible structure size because we are never going to have argument
structures that are larger than a page (I think this was discussed in
the original copy_struct_from_user() patchset thread in late 2019, but I
can't find the reference at the moment.)

I simply forgot to add this when I first submitted openat2, the original
intention was to just match the other syscalls.

> Where is the current -EFAULT for users passing more than a page?
> I only see it for reads beyond the VMA, but not e.g. when checking
> terabytes of zero pages from an anonymous mapping.

I meant that we in practice return -EFAULT if you pass a really large
size (because you end up running off the end of mapped memory). There is
no explicit -EFAULT for large sizes, which is exactly the problem. :P

> 
>     Arnd

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)