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

Re: [PATCH net-next 04/15] net: skbuff: fail segmentation when a segment can't be checksummed


  • To: Josef Bacik <josef@xxxxxxxxxxxxxx>
  • From: Mina Almasry <almasrymina@xxxxxxxxxx>
  • Date: Fri, 23 Oct 2026 07:14:34 -0700
  • Arc-authentication-results: i=1; mx.google.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=UX9XiJ/sX7H9VN90F3Ajif8yPiFt/xrbd1p2Jnn+V8U=; fh=OJ9mylx2JBzyO2BEyMqgd7O+vtLXT4S2AdO6tIKHjWs=; b=oIQouZONqX12qZyFlemZnKWn5rduyx5MPx5AGqCGz+0Y4/9zAPkv8JkGYMZIH3DYFb A3w8+oqMwx/MdYhPC4sSRxwdXd4L9Kk+NG5bZS11YQuKEHJjUuDa+JTYWPP8ehxidJTn 7TJ+X8KgYzk8Qy6V14ZwR7Yxr38cdVRP6gvyIQ78ow1Ld20d7XmPAF7vsLttyx5+EzjL pOtLS3UDlu12J/LQXh8UwO1Y7xtdGjP1/FCt7+3whISIi4bvePbOTSTLS8qzdM0Lt7o9 vF+7Ks8bx8AWTelQAGZ9q6a1EQ7sga4R/DjCK3NTm1psVIzwqDWTy5MkeTGVf0+r8bs9 1biQ==; darn=lists.xenproject.org
  • Arc-seal: i=1; a=rsa-sha256; t=1792764891; cv=none; d=google.com; s=arc-20260327; b=Zh9Clv85s2+wwkxcA77xmuFjXSJqVpOiNMafutAz+LCR5qlaLCSqj2grCyprrg5s3L pm9azRkUuDOcnuikzHK2fX9aOW3+mvVBdW+zL4JDPsYHWSXJfauPvn99MyKwA2bZ6HeN yk5C9blylz62j1MGVQMWIPdeFwNW1tzG0Y9VcMIh4R+PSYd6GyGocZ4+kVJLftoOInpG uqFYKGYwGqhhwus89tg0wj3I0jo1YE52Vy6GGVcqFUSjxroJt8rgMcd0cY3vsFDHdxf8 s5pVbkZr1pBlNRr+WIZhLQJPYRHcmefZuE0YzKIJqL19uKhl7sTf5LuYvpmlIx1mu0A2 Pqaw==
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=20251104 header.d=google.com header.i="@google.com" header.h="Content-Transfer-Encoding:Content-Type:Cc:To:Subject:Message-ID:Date:From:In-Reply-To:References:MIME-Version"
  • Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, Jakub Kicinski <kuba@xxxxxxxxxx>, Paolo Abeni <pabeni@xxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxx>, Andrew Lunn <andrew+netdev@xxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>, Fernando Fernandez Mancera <fmancera@xxxxxxx>, David Laight <david.laight.linux@xxxxxxxxx>, netdev@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 23 Oct 2026 14:14:55 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Oct 19, 2026 at 6:51 AM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> When the device can't checksum the protocol, skb_segment() computes
> each segment's checksum in software with skb_checksum().  With SG on,
> the segments share the original frags, and a devmem TCP skb keeps its
> whole payload in unreadable frags.  The devmem MSG_ZEROCOPY path never
> sets SKBFL_SHARED_FRAG, so the __skb_linearize() fallback is skipped
> too.  skb_checksum() then warns and returns 0, and tcp_gso_segment()
> writes a TCP checksum that doesn't cover the payload that goes out.
>
> That's reachable with tx-checksumming off and SG on: sk_setup_caps()
> still gives TCP SG and HW_CSUM, so the dmabuf send is accepted, and
> harmonize_features() strips the checksum features but keeps SG.
>
> Use skb_checksum_full() and fail the segmentation with -EFAULT when it
> can't checksum a segment, as skb_segment() already does when it can't
> copy one.
>
> Found by the Sashiko AI review of an earlier fix for the same path with
> SG off.
>
> Link: 
> https://lore.kernel.org/all/20261009-b4-skb-segment-nosg-unreadable-v1-1-7e2334a55fe8@xxxxxxxxxxxxxx/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
>  net/core/skbuff.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 40d2dd20d47d..88ed2dd99dc0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5239,9 +5239,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
>                         if (!nskb->remcsum_offload)
>                                 nskb->ip_summed = CHECKSUM_NONE;
> -                       SKB_GSO_CB(nskb)->csum =
> -                               skb_checksum(nskb, doffset,
> -                                            nskb->len - doffset, 0);
> +                       SKB_GSO_CB(nskb)->csum = 0;

So I wonder if this needs to be a generic thing to do insidk of
skb_checksum_full. It seems like the previous patch is complaining
some fields in the skb are no-longer set correctly if we fail
checksumming, like ip_summ. Maybe ->csum as well should not be left
set to non-zero if skb_checksum_full fails.

Anyway,

Reviewed-by: Mina Almasry <almasrymina@xxxxxxxxxx>


-- 
Thanks,
Mina



 


Rackspace

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