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
| ||
|
Message-ID: <1347187.1684403608@warthog.procyon.org.uk> Date: Thu, 18 May 2023 10:53:28 +0100 From: David Howells <dhowells@...hat.com> To: Paolo Abeni <pabeni@...hat.com> Cc: dhowells@...hat.com, netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Willem de Bruijn <willemdebruijn.kernel@...il.com>, David Ahern <dsahern@...nel.org>, Matthew Wilcox <willy@...radead.org>, Al Viro <viro@...iv.linux.org.uk>, Christoph Hellwig <hch@...radead.org>, Jens Axboe <axboe@...nel.dk>, Jeff Layton <jlayton@...nel.org>, Christian Brauner <brauner@...nel.org>, Chuck Lever III <chuck.lever@...cle.com>, Linus Torvalds <torvalds@...ux-foundation.org>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org Subject: Re: [PATCH net-next v7 03/16] net: Add a function to splice pages into an skbuff for MSG_SPLICE_PAGES Paolo Abeni <pabeni@...hat.com> wrote: > Minor nit: please respect the reverse x-mas tree order (there are a few > other occurrences around) I hadn't come across that. Normally I only apply that to the types so that the names aren't all over the place. But whatever. > > + if (space == 0 && > > + !skb_can_coalesce(skb, skb_shinfo(skb)->nr_frags, > > + pages[0], off)) { > > + iov_iter_revert(iter, len); > > + break; > > + } > > It looks like the above condition/checks duplicate what the later > skb_append_pagefrags() will perform below. I guess the above chunk > could be removed? Good point. There used to be an allocation between in the case sendpage_ok() failed and we wanted to copy the data. I've removed that for the moment. > > + ret = -EIO; > > + if (!sendpage_ok(page)) > > + goto out; > > My (limited) understanding is that the current sendpage code assumes > that the caller provides/uses pages suitable for such use. The existing > sendpage_ok() check is in place as way to try to catch possible code > bug - via the WARN_ONCE(). > > I think the same could be done here? Yeah. Okay, I made the attached changes to this patch. David --- diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 56d629ea2f3d..f4a5b51aed22 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -6923,10 +6923,10 @@ static void skb_splice_csum_page(struct sk_buff *skb, struct page *page, ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter *iter, ssize_t maxsize, gfp_t gfp) { + size_t frag_limit = READ_ONCE(sysctl_max_skb_frags); struct page *pages[8], **ppages = pages; - unsigned int i; ssize_t spliced = 0, ret = 0; - size_t frag_limit = READ_ONCE(sysctl_max_skb_frags); + unsigned int i; while (iter->count > 0) { ssize_t space, nr; @@ -6946,20 +6946,13 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter *iter, break; } - if (space == 0 && - !skb_can_coalesce(skb, skb_shinfo(skb)->nr_frags, - pages[0], off)) { - iov_iter_revert(iter, len); - break; - } - i = 0; do { struct page *page = pages[i++]; size_t part = min_t(size_t, PAGE_SIZE - off, len); ret = -EIO; - if (!sendpage_ok(page)) + if (WARN_ON_ONCE(!sendpage_ok(page))) goto out; ret = skb_append_pagefrags(skb, page, off, part,
Powered by blists - more mailing lists