[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH net-next v3 07/15] net: skbuff: let skb_zerocopy() copy a short skb without head_frag



skb_zerocopy() refuses an skb with no head_frag when the caller passes
hlen == 0, because it can't reference such a head as a page.  That check
sits ahead of the small-payload path, which copies everything with
skb_copy_bits() and never references the head.

Open vSwitch gets there with an ordinary request.
OVS_PACKET_CMD_EXECUTE allocates the packet with a kmalloc'd head, a
POP_ETH action on a 14 byte frame leaves it empty, and a USERSPACE
action calls skb_zerocopy(user_skb, skb, 0, 0).  The upcall fails, with
a WARN on CONFIG_DEBUG_NET kernels.  Before
commit 433380f5ab8f ("net: skbuff: don't BUG() on a missing head_frag in
skb_zerocopy()") it hit a BUG().  The request only needs CAP_NET_ADMIN
in the user namespace that owns the network namespace.

Move the check below the small-payload path and document the -EFAULT
case.  A genetlink reproducer run as an unprivileged user in its own
namespaces warns before this patch and doesn't after it.

The small-payload path hands @len to skb_put(), which panics on a
negative value, and nothing runs ahead of it now.  Reject a negative
@len or @hlen on entry with -EINVAL.  Neither caller passes one.

Found by the Sashiko AI review of the BUG_ON() series.

Fixes: 433380f5ab8f ("net: skbuff: don't BUG() on a missing head_frag in 
skb_zerocopy()")
Fixes: af2806f8f90a ("net: Export skb_zerocopy() to zerocopy from one skb to 
another")
Link: 
https://lore.kernel.org/all/179173804997.434549.4450793967361922984@xxxxxxxxxx/
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 net/core/skbuff.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 15d1f4142f29..d79d8463c44e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4072,8 +4072,10 @@ EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
  *     Return value:
  *     0: everything is OK
  *     -ENOMEM: couldn't orphan frags of @from due to lack of memory
- *     -EFAULT: skb_copy_bits() found some problem with skb geometry, or 
readable head
- *      payload would be mixed with unreadable frags.
+ *     -EFAULT: skb_copy_bits() found some problem with skb geometry, @from has
+ *     no head_frag and @hlen is 0 while @len doesn't fit in @to's tailroom,
+ *     or readable head payload would be mixed with unreadable frags.
+ *     -EINVAL: @len or @hlen is negative
  */
 int
 skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
@@ -4084,15 +4086,23 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, 
int len, int hlen)
        struct page *page;
        unsigned int offset;
 
-       if (unlikely(!from->head_frag && !hlen)) {
+       if (unlikely(len < 0 || hlen < 0)) {
                DEBUG_NET_WARN_ON_ONCE(1);
-               return -EFAULT;
+               return -EINVAL;
        }
 
        /* dont bother with small payloads */
        if (len <= skb_tailroom(to))
                return skb_copy_bits(from, 0, skb_put(to, len), len);
 
+       /* Without a head_frag the head can't be referenced as a page, so the
+        * caller has to ask for it to be copied.
+        */
+       if (unlikely(!from->head_frag && !hlen)) {
+               DEBUG_NET_WARN_ON_ONCE(1);
+               return -EFAULT;
+       }
+
        to->pp_recycle = from->pp_recycle;
        if (hlen) {
                ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen);

-- 
2.55.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.