Re: [PATCH v9 1/9] iov_iter: add copy_struct_from_iter()

From: Omar Sandoval <osandov@osandov.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Aleksa Sarai <cyphar@cyphar.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Dave Chinner <david@fromorbit.com>, Jann Horn <jannh@google.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Linux API <linux-api@vger.kernel.org>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH v9 1/9] iov_iter: add copy_struct_from_iter()
Date: Fri, 2 Apr 2021 10:50:50 -0700	[thread overview]
Message-ID: <YGdZeh4K3BxQPcGx@relinquished.localdomain> (raw)
In-Reply-To: <20210402080423.t26zd34p2oxbzvuj@wittgenstein>

On Fri, Apr 02, 2021 at 10:04:23AM +0200, Christian Brauner wrote:
> On Fri, Apr 02, 2021 at 12:33:20AM -0700, Omar Sandoval wrote:
> > On Thu, Apr 01, 2021 at 09:05:22AM -0700, Linus Torvalds wrote:
> > > On Wed, Mar 31, 2021 at 11:51 PM Omar Sandoval <osandov@osandov.com> wrote:
> > > >
> > > > + *
> > > > + * The recommended usage is something like the following:
> > > > + *
> > > > + *     if (usize > PAGE_SIZE)
> > > > + *       return -E2BIG;
> > > 
> > > Maybe this should be more than a recommendation, and just be inside
> > > copy_struct_from_iter(), because otherwise the "check_zeroed_user()"
> > > call might be quite the timesink for somebody who does something
> > > stupid.
> > 
> > I did actually almost send this out with the check in
> > copy_struct_from_iter(), but decided not to for consistency with
> > copy_struct_from_user().
> > 
> > openat2() seems to be the only user of copy_struct_from_user() that
> > doesn't limit to PAGE_SIZE, which is odd given that Aleksa wrote both
> 
> Al said there's nothing wrong with copying large chunks of memory so we
> shouldn't limit the helper but instead limit the callers which have
> expectations about their size limit:
> https://lore.kernel.org/lkml/20190905182801.GR1131@ZenIV.linux.org.uk/

Thanks for the context. So I guess it makes sense to keep the check
"recommended" for both functions.