netdev - Re: Reorganising how the networking layer handles memory

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aBoVAd-XX_44RKbC@infradead.org>
Date: Tue, 6 May 2025 06:56:17 -0700
From: Christoph Hellwig <hch@...radead.org>
To: David Howells <dhowells@...hat.com>
Cc: Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>,
	David Hildenbrand <david@...hat.com>,
	John Hubbard <jhubbard@...dia.com>,
	Christoph Hellwig <hch@...radead.org>, willy@...radead.org,
	netdev@...r.kernel.org, linux-mm@...ck.org,
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: Reorganising how the networking layer handles memory

On Tue, May 06, 2025 at 02:50:49PM +0100, David Howells wrote:
> > > (2) sendmsg(MSG_ZEROCOPY) suffers from the O_DIRECT vs fork() bug because
> > >      it doesn't use page pinning.  It needs to use the GUP routines.
> > 
> > We end up calling iov_iter_get_pages2(). Is it not setting
> > FOLL_PIN is a conscious choice, or nobody cared until now?
> 
> iov_iter_get_pages*() predates GUP, I think.

It predates pin_user_pages, but get_user_pages is much older.

> There's now an
> iov_iter_extract_pages() that does the pinning stuff, but you have to do a
> different cleanup, which is why I created a new API call.

But yes, iov_iter_get_pages* needs to go away in favour of
iov_iter_extract_pages, and I'm still annoyed that despite multiple
pings no one has done any work on that outside of block / block based
direct I/O and netfs.

> > >  (3) sendmsg(MSG_SPLICE_PAGES) isn't entirely satisfactory because it can't be
> > >      used with certain memory types (e.g. slab).  It takes a ref on whatever
> > >      it is given - which is wrong if it should pin this instead.
> > 
> > s/takes a ref/requires a ref/ ? I mean - the caller implicitly grants 
> > a ref  to the stack, right? But yes, the networking stack will try to
> > release it.
> 
> I mean 'takes' as in skb_append_pagefrags() calls get_page() - something that
> needs to be changed.
> 
> Christoph Hellwig would like to make it such that the extractor gets
> {phyaddr,len} rather than {page,off,len} - so all you, the network layer, see
> is that you've got a span of memory to use as your buffer.  How that span of
> memory is managed is the responsibility of whoever called sendmsg() - and they
> need a callback to be able to handle that.

Not sure what the extractor is, but we plan to change the bio_vec
to be physical address instead of page+offset based.  Where we is
a lot more people than just me.

> Once advantage of delegating it to the caller, though, and having the caller
> keep track of which bits in still needs to hold on to by transmission
> completion position is that we don't need to manage refs/pins across sk_buff
> duplication - let alone what we should do with stuff that's kmalloc'd.

And the callers already do that for all other kinds of I/O anyway.