linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Laight <David.Laight@aculab.com>,
	Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 03/11] fs: add new read_uptr and write_uptr file operations
Date: Mon, 29 Jun 2020 17:29:12 +0200	[thread overview]
Message-ID: <20200629152912.GA26172@lst.de> (raw)
In-Reply-To: <CAHk-=wjxQczqZ96esvDrH5QZsLg6azXCGDgo+Bmm6r8t2ssasg@mail.gmail.com>

On Sat, Jun 27, 2020 at 09:33:03AM -0700, Linus Torvalds wrote:
> I thought there was just one very specific case of "oh, in certain
> cases of setsockopt we don't know what size this address is and optlen
> is ignored", so we have to just pass the pointer down to the protocol,
> which is the point that knows how much of an address it wants..

The setsock issue is a little more complicated.  Let me try to summarize
it:

 - setsock takes a (user) pointer and len
 - unfortunately while the designed of the BSD socket API designed the
   len to be correct some protocol implementations have been sloppy
   and just use a hardcoded len for the value plus some other funnies
 - unfortunately there is some BPF magic that can attach to a socket
   and be run, and that (and only that in the latest kernel) can cause
   a setsockopt to take a kernel buffer.  One that was copied from
   userspace earlier and had the BPF program run on it.
 - unfortunately we have about 90 ->setsockopt instances, and the BPF
   hook is not specific to one particular of them.  In fact the
   BPF program can run for options that don't even exist, and based on
   my previous dicussion Facebook has setups that rely on that.

> Was that a misunderstanding on my part?
> 
> Because if there are tons and tons of places that want this "either
> kernel or user" then we could still have a helper function for it, but
> it means that the whole "limit the cases" advantage to some degree
> goes away.

But except for setsockopt we don't really have anything like that left.
There is some alpha arch code that would need to be duplicated for
user vs kernel pointers, but I suspect it will get cleaner by that,
and the messy s390 crypto driver whіch will be a bit of work, but all
internal to that driver.

So based on that I'd rather get away without our flag and tag the
kernel pointer case in setsockopt explicitly.

  parent reply	other threads:[~2020-06-29 19:07 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 16:28 [RFC] stop using ->read and ->write for kernel access Christoph Hellwig
2020-06-24 16:28 ` [PATCH 01/11] uptr: add a new "universal pointer" type Christoph Hellwig
2020-06-24 16:28 ` [PATCH 02/11] fs: factor out a set_fmode_can_read_write helper Christoph Hellwig
2020-06-24 16:28 ` [PATCH 03/11] fs: add new read_uptr and write_uptr file operations Christoph Hellwig
2020-06-24 17:19   ` Linus Torvalds
2020-06-24 17:55     ` Christoph Hellwig
2020-06-24 18:11       ` Linus Torvalds
2020-06-24 18:14         ` Christoph Hellwig
2020-06-24 18:20           ` Linus Torvalds
2020-06-24 18:24             ` Christoph Hellwig
2020-06-24 18:29               ` Matthew Wilcox
2020-06-24 18:31                 ` Christoph Hellwig
2020-06-24 18:15         ` Linus Torvalds
2020-06-27 10:49         ` David Laight
2020-06-27 16:33           ` Linus Torvalds
2020-06-29  8:21             ` David Laight
2020-06-29 15:29             ` Christoph Hellwig [this message]
2020-06-29 17:02               ` Linus Torvalds
2020-06-29 18:07                 ` Christoph Hellwig
2020-06-29 18:29                   ` Linus Torvalds
2020-06-29 18:36                     ` Christoph Hellwig
2020-06-29 19:10                       ` Linus Torvalds
2020-06-30  7:04                         ` Christoph Hellwig
2020-06-30  7:51                 ` David Laight
2020-07-08  5:14             ` Luis Chamberlain
2020-06-24 17:56     ` Matthew Wilcox
2020-06-24 17:59       ` Christoph Hellwig
2020-06-24 18:37         ` Christoph Hellwig
2020-06-24 18:43           ` Matthew Wilcox
2020-06-24 16:28 ` [PATCH 04/11] sysctl: switch to ->{read,write}_uptr Christoph Hellwig
2020-06-24 16:28 ` [PATCH 05/11] fs: refactor new_sync_read Christoph Hellwig
2020-06-24 16:28 ` [PATCH 06/11] proc: add a read_iter method to proc proc_ops Christoph Hellwig
2020-06-24 16:28 ` [PATCH 07/11] seq_file: add seq_read_iter Christoph Hellwig
2020-06-24 16:28 ` [PATCH 09/11] proc: switch over direct seq_read method calls to seq_read_iter Christoph Hellwig
2020-06-24 16:29 ` [PATCH 10/11] fs: don't allow kernel reads and writes using ->read and ->write Christoph Hellwig
2020-06-24 16:29 ` [PATCH 11/11] fs: don't allow splice read/write without explicit ops Christoph Hellwig

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=20200629152912.GA26172@lst.de \
    --to=hch@lst.de \
    --cc=David.Laight@aculab.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yzaikin@google.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).