Re: [PATCH 0/3] vmsplice: make vmsplice a trivial wrapper for preadv2/pwritev2
Andy Lutomirski <[email protected]>
| Newsgroups | org.kernel.vger.linux-api,dev.linux.lists.patches,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kvack.linux-mm |
|---|---|
| Message-ID | <CALCETrXpqPMS487Bm8f8mHe8hv9DzCqoaW4UdoHetzYBUAhYLw@mail.gmail.com> |
On Wed, Jun 3, 2026 at 2:39 PM Linus Torvalds <[email protected]> wrote: > > On Wed, 3 Jun 2026 at 14:31, Andy Lutomirski <[email protected]> wrote: > > > > I think I buried the lede too much and you're arguing against what I > > was trying not to say. > > > > Maybe we should keep an API that does an optimized copy, from one fd > > to another, that can send from a file to the network with at most ONE > > cpu-side copy. Not aiming for zero like sendfile / splice. Aiming > > for one. > > Oh, absolutely - that's what my completely untested test patch basically did. > > The user space interface was still there. > > And the networking side still continued to use the ->splice_write() > thing for writing to the socket. So I'm suspicious that you've possibly make bugs much (MUCH) harder to exploit, but the underlying awful code and opportunity for bugs is still there. MSG_SPLICE_PAGES is still around, and there is still (AFAICS) no actual coherent description of what it means. There is code that checks for it and apparently needs to do something special. Foir example, some random kernel version I have checked out has this delight in af_alg.c: /* use the existing memory in an allocated page */ if (ctx->merge && !(msg->msg_flags & MSG_SPLICE_PAGES)) { Grepping for MSG_SPLICE_PAGES come up with all kinds of terrors. Check out the lovely comment in drivers/block/drbd/drbd_main.c, for example... And even with your patch, I think checking for MSG_SPLICE_PAGES still matters: if I write to a pipe (using copy_splice_read or even just plain write) and then I tee() that data, then I splice one of those teed copies into a socket, then we hit ->sendmsg with MSG_SPLICE_PAGES set, and we're hoping that the code does the right thing. And maybe all the bugs are fixed by now or maybe they're not. Most of what your patch accomplishes is breaking the connection between the buffers and pagecache, so you can't poison /sbin/su. It also seems kind of unfortunate that we can have skbs that contain data that isn't actually owned by the socket in question, and, with your patch applied, I'm wondering if the only case where this can really happen is tee() and a handful of random drivers that send to sockets. (The ones in drivers/nvme/host/tcp.c and iSCSI seem like the ones that people are likely to care about the most.) I *think* that what I'm sort of suggesting is to drop this ability from the kernel as well, or at least to consider it. skbs would always own their contents. And something would get wired up so that at least the cases of sendfile, nvme and iscsi to TCP or UDP sockets would still works with only one copy, from the source page cache into the socket buffer. I suppose the counterargument is that, even if more bugs exist, it's a bit hard to imagine a real attack involving tee, and one needs privileges to set up nvme or iscsi aimed at an unusual socket type. -- Andy Lutomirski AMA Capital Management, LLC