linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Aleksa Sarai" <cyphar@cyphar.com>
Cc: "Ingo Molnar" <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>,
	"Dietmar Eggemann" <dietmar.eggemann@arm.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Benjamin Segall" <bsegall@google.com>,
	"Mel Gorman" <mgorman@suse.de>,
	"Valentin Schneider" <vschneid@redhat.com>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>, shuah <shuah@kernel.org>,
	"Kees Cook" <kees@kernel.org>,
	"Florian Weimer" <fweimer@redhat.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Linux-Arch <linux-arch@vger.kernel.org>,
	linux-kselftest@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH RFC 3/8] openat2: explicitly return -E2BIG for (usize > PAGE_SIZE)
Date: Mon, 02 Sep 2024 19:23:47 +0000	[thread overview]
Message-ID: <0455ebf7-3f84-44c7-84b3-9ed6e218cdc0@app.fastmail.com> (raw)
In-Reply-To: <20240902.160305-cuddly.doc.quaint.provider-RsRaXpw78cll@cyphar.com>

On Mon, Sep 2, 2024, at 16:08, Aleksa Sarai wrote:
>> >  	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.

Ok, I see. I guess it makes sense to keep this one consistent with the
other ones, but we may want to revisit this in the future and
come up with something that is independent of CONFIG_PAGE_SIZE.

>> 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

Got it, thanks.

     Arnd

  reply	other threads:[~2024-09-02 19:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-02  7:06 [PATCH RFC 0/8] extensible syscalls: CHECK_FIELDS to allow for easier feature detection Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 1/8] uaccess: add copy_struct_to_user helper Aleksa Sarai
2024-09-02  8:55   ` Arnd Bergmann
2024-09-02 16:02     ` Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 2/8] sched_getattr: port to copy_struct_to_user Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 3/8] openat2: explicitly return -E2BIG for (usize > PAGE_SIZE) Aleksa Sarai
2024-09-02  9:09   ` Arnd Bergmann
2024-09-02 16:08     ` Aleksa Sarai
2024-09-02 19:23       ` Arnd Bergmann [this message]
2024-09-02  7:06 ` [PATCH RFC 4/8] openat2: add CHECK_FIELDS flag to usize argument Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 5/8] clone3: " Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 6/8] selftests: openat2: add 0xFF poisoned data after misaligned struct Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 7/8] selftests: openat2: add CHECK_FIELDS selftests Aleksa Sarai
2024-09-02  7:06 ` [PATCH RFC 8/8] selftests: clone3: " Aleksa Sarai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0455ebf7-3f84-44c7-84b3-9ed6e218cdc0@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=brauner@kernel.org \
    --cc=bsegall@google.com \
    --cc=cyphar@cyphar.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=juri.lelli@redhat.com \
    --cc=kees@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).