All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: Sultan Alsawaf <sultan@kerneltoast.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, kerneljasonxing@gmail.com,
	kuniyu@google.com, mhal@rbox.co, jiayuan.chen@linux.dev,
	steffen.klassert@secunet.com, ben@decadent.org.uk,
	herbert@gondor.apana.org.au, dsahern@kernel.org,
	sd@queasysnail.net, netdev@vger.kernel.org,
	stable@vger.kernel.org, imv4bel@gmail.com
Subject: Re: [PATCH net v4] net: skbuff: propagate shared-frag marker through frag-transfer helpers
Date: Fri, 15 May 2026 15:36:59 +0900	[thread overview]
Message-ID: <aga_C6fXL0dZdDzb@v4bel> (raw)
In-Reply-To: <aga8lH0sgneYCCgY@sultan-box>

On Thu, May 14, 2026 at 11:26:28PM -0700, Sultan Alsawaf wrote:
> On Fri, May 15, 2026 at 02:55:35PM +0900, Hyunwoo Kim wrote:
> > Changes in v4:
> > - Include the tcp_clone_payload() propagation suggested by Sabrina.
> > - Drop the skb_try_coalesce() change; addressed by commit f84eca581739.
> > - v3: https://lore.kernel.org/all/agW4vC0r8QOUKtRT@v4bel/
> > 
> > Changes in v3:
> > - Include the skb_gro_receive() audit patch suggested by Sultan
> > - v2: https://lore.kernel.org/all/agToIEDI4TaTNLRb@v4bel/
> > 
> > Changes in v2:
> > - Also propagate SHARED_FRAG in skb_try_coalesce() and skb_shift()
> > - v1: https://lore.kernel.org/all/agRfuVOeMI5pbHhY@v4bel/
> 
> Hi Hyunwoo,
> 
> Per your ask to me to use AI for exploring relevant paths [1], I've attached my
> findings from a pretty thorough day of hunting for these with Claude.
> 
> None of the findings appear to be currently exploitable.
> 
> Please let me know if you have any questions, and I hope you find this helpful.
> 
> [1] https://lore.kernel.org/all/agWUdie1xBvBu22I@v4bel/
> 
> Thanks,
> Sultan

Thank you so much. This is a really useful report; I'll take it and dig 
further from there.

Thanks again!


Best regards,
Hyunwoo Kim


> Line numbers below are against netdev commit 5db89c99566fc ("net: ifb: report
> ethtool stats over num_tx_queues").
> 
> We audited the netdev tree for remaining sites where frag descriptors are
> transferred between skbs without propagating SKBFL_SHARED_FRAG.  Hyunwoo Kim's
> v4 fix covers __pskb_copy_fclone, skb_shift, skb_gro_receive,
> skb_gro_receive_list, and tcp_clone_payload; the standalone f84eca5817390
> covers skb_try_coalesce.  Several other sites in newer code have the same class
> of bug.
> 
> None of these are currently reachable for page-cache corruption, since each one
> is blocked by independent guards (cloned skbs, TX-only paths, or data copying).
> They should still be fixed for defense-in-depth: skb_copy_header() doesn't
> propagate shinfo->flags, so every frag-transfer helper that allocates new
> shinfo needs its own propagation line.  This guarantees the bug class will
> recur whenever someone writes a new helper without realizing it.
> 
> --- Findings ---
> 
> 1. unix_stream_sendmsg() -- net/unix/af_unix.c:2461
> 
>    Calls skb_splice_from_iter() with MSG_SPLICE_PAGES but never sets
>    SKBFL_SHARED_FRAG.  It's the only skb_splice_from_iter() caller that
>    doesn't do so; compare with tcp_sendmsg_locked() at tcp.c:1371,
>    ip_append_data() at ip_output.c:1237, and ip6_append_data() at
>    ip6_output.c:1801.
> 
>    Not reachable since AF_UNIX skbs don't enter the network stack.  When
>    forwarded via splice (unix -> pipe -> tcp), the destination protocol's
>    sendmsg sets the flag independently.
> 
>    Fix: add the same flag-set after skb_splice_from_iter(), matching the TCP
>    pattern.
> 
> 2. iptfs_consume_frags() -- net/xfrm/xfrm_iptfs.c:2152
> 
>    memcpy() of the frag array plus iptfs_skb_head_to_frag() conversion.  Zero
>    references to SKBFL_SHARED_FRAG in the entire 2700-line file.
> 
>    Not reachable due to three independent guards: the fragmentation path copies
>    data into linear via skb_copy_seq_read() + skb_put() (no page-cache frag
>    references in the result), the share_ok guard blocks aggregation for TCP
>    skbs (since tcp_stream_alloc_skb() uses alloc_skb_fclone() which doesn't
>    set head_frag), and simple aggregation fails because the base skb is a TCP
>    clone.
> 
> 3. iptfs_skb_add_frags() -- net/xfrm/xfrm_iptfs.c:458
> 
>    *tofrag = *frag + __skb_frag_ref() without flag propagation.  The frag walk
>    struct doesn't carry source flags.
> 
>    RX path frags come from NIC RX buffers (not page cache).  TX path has the
>    same guards as iptfs_consume_frags().
> 
> 4. tcp_clone_payload() -- net/ipv4/tcp_output.c:2607
>    **Now fixed in v4** (suggested by Sabrina Dubroca).
> 
>    skb_frag_page_copy() / skb_frag_off_copy() / skb_frag_size_set() +
>    skb_frag_ref() from write-queue skbs to a new MTU probe skb.  No flag
>    propagation.
> 
>    TX-only (called by tcp_mtu_probe()).  The probe skb goes to
>    tcp_transmit_skb() which clones it before sending.  Can't reach ESP input.
> 
> 5. skb_zerocopy() -- net/core/skbuff.c:3843
> 
>    Frag descriptor assignment + skb_frag_ref().  Calls skb_zerocopy_clone()
>    which handles the zerocopy uarg but not SKBFL_SHARED_FRAG.
> 
>    All callers (nfnetlink_queue, openvswitch) send the copy to userspace via
>    netlink.  The original skb continues through the stack with its flags
>    intact.
> 
> 6. chcr_ktls_copy_record_in_skb() -- drivers/.../chcr_ktls.c:1654
> 
>    Frag descriptor assignment from TLS record + __skb_frag_ref().  No flag
>    propagation.
> 
>    TX-only, hardware-specific (Chelsio T6 kTLS offload).
> 
> 7. esp_output_head() -- net/ipv4/esp4.c:426
> 
>    The output-side skip_cow checks !skb_cloned() but never checks
>    !skb_has_shared_frag().  Compare with esp_input() at line 877 which does
>    check it (CVE-2026-43284).  The first skip_cow path (tailen <=
>    skb_tailroom) keeps inplace=true, so AEAD encrypt would write ciphertext
>    over source SG entries including frag pages.
> 
>    Not reachable in practice: kretprobe tracing on a booted 7.1.0-rc3 kernel
>    confirmed esp_output_head() always returns nfrags >= 2 (the inplace=false
>    second branch), never nfrags=1.  For paged skbs from splice, the tailroom
>    is insufficient for the ESP trailer.  The inplace=false path allocates
>    separate output pages, so frag data is only read as source, never written.
> 
>    esp_output_head() should still add the !skb_has_shared_frag() check to
>    match esp_input(), since a future change to skb allocation sizing could
>    make the first skip_cow path reachable.
> 
> --- Root cause ---
> 
> skb_copy_header() copies gso_size / gso_segs / gso_type from old shinfo to
> new, but it never copies shinfo->flags.  As a result, every frag-transfer
> helper that allocates new shinfo needs its own explicit flag propagation.  This
> is easy to miss when writing new helpers, which is how we ended up with seven
> independent instances of the same bug.
> 
> A potential long-term fix would be to propagate SKBFL_SHARED_FRAG (and
> SKBFL_PURE_ZEROCOPY) inside skb_copy_header() itself, matching how skb_split()
> already handles both flags.  This would eliminate the bug class at the source
> rather than playing whack-a-mole with each new helper.


  reply	other threads:[~2026-05-15  6:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  5:55 [PATCH net v4] net: skbuff: propagate shared-frag marker through frag-transfer helpers Hyunwoo Kim
2026-05-15  6:07 ` Hyunwoo Kim
2026-05-15  6:26 ` Sultan Alsawaf
2026-05-15  6:36   ` Hyunwoo Kim [this message]
2026-05-15 15:11     ` Hyunwoo Kim
2026-05-15 16:41 ` Aaron Esau
2026-05-15 20:22   ` Hyunwoo Kim
2026-05-15 21:36     ` Hyunwoo Kim
2026-05-16  0:08       ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aga_C6fXL0dZdDzb@v4bel \
    --to=imv4bel@gmail.com \
    --cc=ben@decadent.org.uk \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=horms@kernel.org \
    --cc=jiayuan.chen@linux.dev \
    --cc=kerneljasonxing@gmail.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=mhal@rbox.co \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sd@queasysnail.net \
    --cc=stable@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    --cc=sultan@kerneltoast.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.