[PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[Eric Dumazet]

pskb_carve_inside_header() and pskb_carve_inside_nonlinear() remove
the first bytes of a packet and reallocate skb->head.

All the headers that were present before the operation are gone,
but both functions call skb_headers_offset_update(skb, 0), which
is a no-op : skb->mac_header, skb->network_header,
skb->transport_header and skb->csum_start keep their old values and
now describe bytes which are no longer there.

Both helpers size the new head from the old skb_end_offset(), so the
stale offsets still land inside the new allocation. They point past
skb_tail_pointer() though, to bytes that were never initialized.

pskb_carve_inside_nonlinear() is the worst case, because it leaves a
zombie skb with an empty linear part (skb->data ==
skb_tail_pointer(skb), skb_headlen(skb) == 0), while
skb_mac_header_was_set() is still true and skb->mac_header is way
ahead of skb->data.

The only user of pskb_extract() is rds_tcp_data_recv(), and the
carved skb is queued on tinc->ti_skb_list. When the RDS incoming
message is released, rds_tcp_inc_free() calls skb_queue_purge(),
which frees the skbs with SKB_DROP_REASON_QUEUE_PURGE. This is
visible from drop_monitor, which then tries to pull back to the
(bogus) mac header :

skbuff: __skb_pull(len=234)
skb len=6968 data_len=6968 headroom=0 headlen=0 tailroom=0
end-tail=384 mac=(234,14) mac_len=14 net=(248,40) trans=288
shinfo(txflags=0 nr_frags=1 gso(size=1428 type=16 segs=5))
csum(0x100120 start=288 offset=16 ip_summed=3 complete_sw=0 valid=1 level=0)
hash(0x7b446c6c sw=0 l4=1) proto=0x86dd pkttype=0 iif=60
------------[ cut here ]------------
kernel BUG at ./include/linux/skbuff.h:2847!

Add skb_carve_reset_headers() to mark the mac and transport headers
as not set, reset the network header, clear skb->mac_len, and drop
a now meaningless CHECKSUM_PARTIAL (csum_start no longer describes
anything).

Invalidate the inner offsets as well. Unlike mac_header and
transport_header they have no "unset" sentinel, so a leftover
non-zero value still looks like a real header. Zero
skb->inner_mac_header, skb->inner_network_header,
skb->inner_transport_header, skb->inner_protocol and
skb->encapsulation, so that all the header state is invalidated in
one place.

v2: fixed an inaccurate changelog. The stale offsets stay inside the
    new skb->head, which is never smaller than the old one, they
    simply point past skb_tail_pointer() to bytes that are gone.
    Thanks to Xuanqiang Luo for insisting on this.
    Also invalidate the inner header state, as suggested by the
    netdev AI review :
    https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260911114922.621937-1-edumazet%40google.com

Fixes: 6fa01ccd8830 ("skbuff: Add pskb_extract() helper function")
Reported-by: syzbot+586af68eb819833c2d91@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6aa3e9d3.f2639fcc.29487d.0028.GAE@google.com/
Cc: Xuanqiang Luo <xuanqiang.luo@linux.dev>
Cc: Allison Henderson <achender@kernel.org>
Cc: rds-devel@oss.oracle.com
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/skbuff.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cc3b4b70288b4e3f17984cd4f4e4b7a330353e15..609f2c7f4a47ad7a81426af149727fa47be12bb5 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6832,6 +6832,34 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,
 }
 EXPORT_SYMBOL(alloc_skb_with_frags);
 
+/* pskb_carve_inside_header() and pskb_carve_inside_nonlinear()
+ * remove the first bytes of a packet and reallocate skb->head.
+ *
+ * Whatever headers were present before the operation are gone,
+ * we must not leave stale offsets, otherwise users of this skb
+ * (skb_dump(), drop_monitor, taps, ...) would read or pull garbage.
+ */
+static void skb_carve_reset_headers(struct sk_buff *skb)
+{
+	skb_unset_mac_header(skb);
+	skb_unset_transport_header(skb);
+	skb_reset_network_header(skb);
+	skb->mac_len = 0;
+
+	/* Inner offsets have no "unset" marker, zero them so that
+	 * skb_inner_network_header_was_set() becomes false and no
+	 * consumer mistakes them for a real (and long gone) header.
+	 */
+	skb->inner_mac_header = 0;
+	skb->inner_network_header = 0;
+	skb->inner_transport_header = 0;
+	skb->inner_protocol = 0;
+	skb->encapsulation = 0;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		skb->ip_summed = CHECKSUM_NONE;
+}
+
 /* carve out the first off bytes from skb when off < headlen */
 static int pskb_carve_inside_header(struct sk_buff *skb, const u32 off,
 				    const int headlen, gfp_t gfp_mask)
@@ -6887,7 +6915,7 @@ static int pskb_carve_inside_header(struct sk_buff *skb, const u32 off,
 	skb->head_frag = 0;
 	skb_set_end_offset(skb, size);
 	skb_set_tail_pointer(skb, skb_headlen(skb));
-	skb_headers_offset_update(skb, 0);
+	skb_carve_reset_headers(skb);
 	skb->cloned = 0;
 	skb->hdr_len = 0;
 	skb->nohdr = 0;
@@ -7027,7 +7055,7 @@ static int pskb_carve_inside_nonlinear(struct sk_buff *skb, const u32 off,
 	skb->data = data;
 	skb_set_end_offset(skb, size);
 	skb_reset_tail_pointer(skb);
-	skb_headers_offset_update(skb, 0);
+	skb_carve_reset_headers(skb);
 	skb->cloned   = 0;
 	skb->hdr_len  = 0;
 	skb->nohdr    = 0;
-- 
2.55.0.1032.g73a4cd73de-goog

Re: [PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[Xuanqiang Luo]

在 2026/9/15 21:04, Eric Dumazet 写道:
> pskb_carve_inside_header() and pskb_carve_inside_nonlinear() remove
> the first bytes of a packet and reallocate skb->head.
> 
> All the headers that were present before the operation are gone,
> but both functions call skb_headers_offset_update(skb, 0), which
> is a no-op : skb->mac_header, skb->network_header,
> skb->transport_header and skb->csum_start keep their old values and
> now describe bytes which are no longer there.
> 
> Both helpers size the new head from the old skb_end_offset(), so the
> stale offsets still land inside the new allocation. They point past
> skb_tail_pointer() though, to bytes that were never initialized.
> 
> pskb_carve_inside_nonlinear() is the worst case, because it leaves a
> zombie skb with an empty linear part (skb->data ==
> skb_tail_pointer(skb), skb_headlen(skb) == 0), while
> skb_mac_header_was_set() is still true and skb->mac_header is way
> ahead of skb->data.
> 
> The only user of pskb_extract() is rds_tcp_data_recv(), and the
> carved skb is queued on tinc->ti_skb_list. When the RDS incoming
> message is released, rds_tcp_inc_free() calls skb_queue_purge(),
> which frees the skbs with SKB_DROP_REASON_QUEUE_PURGE. This is
> visible from drop_monitor, which then tries to pull back to the
> (bogus) mac header :
> 
> skbuff: __skb_pull(len=234)
> skb len=6968 data_len=6968 headroom=0 headlen=0 tailroom=0
> end-tail=384 mac=(234,14) mac_len=14 net=(248,40) trans=288
> shinfo(txflags=0 nr_frags=1 gso(size=1428 type=16 segs=5))
> csum(0x100120 start=288 offset=16 ip_summed=3 complete_sw=0 valid=1 level=0)
> hash(0x7b446c6c sw=0 l4=1) proto=0x86dd pkttype=0 iif=60
> ------------[ cut here ]------------
> kernel BUG at ./include/linux/skbuff.h:2847!
> 
> Add skb_carve_reset_headers() to mark the mac and transport headers
> as not set, reset the network header, clear skb->mac_len, and drop
> a now meaningless CHECKSUM_PARTIAL (csum_start no longer describes
> anything).
> 
> Invalidate the inner offsets as well. Unlike mac_header and
> transport_header they have no "unset" sentinel, so a leftover
> non-zero value still looks like a real header. Zero
> skb->inner_mac_header, skb->inner_network_header,
> skb->inner_transport_header, skb->inner_protocol and
> skb->encapsulation, so that all the header state is invalidated in
> one place.
> 
> v2: fixed an inaccurate changelog. The stale offsets stay inside the
>      new skb->head, which is never smaller than the old one, they
>      simply point past skb_tail_pointer() to bytes that are gone.
>      Thanks to Xuanqiang Luo for insisting on this.
>      Also invalidate the inner header state, as suggested by the
>      netdev AI review :
>      https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260911114922.621937-1-edumazet%40google.com
> 
> Fixes: 6fa01ccd8830 ("skbuff: Add pskb_extract() helper function")
> Reported-by: syzbot+586af68eb819833c2d91@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6aa3e9d3.f2639fcc.29487d.0028.GAE@google.com/
> Cc: Xuanqiang Luo <xuanqiang.luo@linux.dev>
> Cc: Allison Henderson <achender@kernel.org>
> Cc: rds-devel@oss.oracle.com
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Thanks!

Re: [PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Incomplete invalidation of checksum metadata in the new
  skb_carve_reset_headers() (net/core/skbuff.c:6842).

--

net: skbuff: do not leave stale header offsets after pskb_carve()

The two pskb_carve() helpers used to call skb_headers_offset_update(skb, 0),
which left mac/network/transport offsets describing bytes that were just
removed. This patch adds skb_carve_reset_headers() to unset the mac and
transport headers, reset the network header, clear mac_len, zero the inner
header state, and downgrade CHECKSUM_PARTIAL to CHECKSUM_NONE.

Closes: https://lore.kernel.org/netdev/6aa3e9d3.f2639fcc.29487d.0028.GAE@google.com/

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index cc3b4b70288b4..609f2c7f4a47a 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6832,6 +6832,34 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,
>  }
>  EXPORT_SYMBOL(alloc_skb_with_frags);
>  
> +/* pskb_carve_inside_header() and pskb_carve_inside_nonlinear()
> + * remove the first bytes of a packet and reallocate skb->head.
> + *
> + * Whatever headers were present before the operation are gone,
> + * we must not leave stale offsets, otherwise users of this skb
> + * (skb_dump(), drop_monitor, taps, ...) would read or pull garbage.
> + */
> +static void skb_carve_reset_headers(struct sk_buff *skb)
> +{
> +	skb_unset_mac_header(skb);
> +	skb_unset_transport_header(skb);
> +	skb_reset_network_header(skb);
> +	skb->mac_len = 0;
> +
> +	/* Inner offsets have no "unset" marker, zero them so that
> +	 * skb_inner_network_header_was_set() becomes false and no
> +	 * consumer mistakes them for a real (and long gone) header.
> +	 */
> +	skb->inner_mac_header = 0;
> +	skb->inner_network_header = 0;
> +	skb->inner_transport_header = 0;
> +	skb->inner_protocol = 0;
> +	skb->encapsulation = 0;
> +
> +	if (skb->ip_summed == CHECKSUM_PARTIAL)
> +		skb->ip_summed = CHECKSUM_NONE;
> +}

[Severity: Medium]
The changelog says the goal is that "all the header state is invalidated in
one place".  Is the checksum state fully invalidated here?

skb_carve_reset_headers() only rewrites ip_summed, while skb->csum,
skb->csum_valid, skb->csum_complete_sw, skb->csum_level and
skb->csum_not_inet keep the values they had before the bytes were removed.

For the CHECKSUM_PARTIAL case, the syzbot dump quoted in the changelog shows
the incoming skb already has valid=1:

  csum(0x100120 start=288 offset=16 ip_summed=3 complete_sw=0 valid=1 level=0)

After the downgrade this becomes ip_summed=CHECKSUM_NONE with csum_valid
still 1, and skb_csum_unnecessary() in include/linux/skbuff.h returns true on
csum_valid alone:

  static inline int skb_csum_unnecessary(const struct sk_buff *skb)
  {
	return ((skb->ip_summed == CHECKSUM_UNNECESSARY) ||
		skb->csum_valid ||
		(skb->ip_summed == CHECKSUM_PARTIAL &&
		 skb_checksum_start_offset(skb) >= 0));
  }

Can a consumer of the carved skb then believe the payload was already
verified, even though the bytes the checksum covered are gone?  A leftover
csum_not_inet would similarly still claim CRC32c semantics, and skb_dump()
would print the contradictory "ip_summed=0 ... valid=1".

Should csum_valid, csum_complete_sw, csum_level and csum_not_inet be cleared
in the same helper?

The CHECKSUM_COMPLETE case looks untouched as well.  Unlike
skb_pull_rcsum()/skb_postpull_rcsum(), the carve path never subtracts the off
bytes removed from the front from skb->csum, and pskb_extract() trims the tail
with plain pskb_trim() rather than pskb_trim_rcsum():

  net/core/skbuff.c:pskb_extract() {
	...
	if (pskb_carve(clone, off, gfp) < 0 ||
	    pskb_trim(clone, to_copy)) {
	...
  }

So a CHECKSUM_COMPLETE skb keeps a whole-packet checksum that no longer
matches its contents.  Would it make sense to also drop or fix up
CHECKSUM_COMPLETE in skb_carve_reset_headers(), or to use pskb_trim_rcsum()
in pskb_extract()?

The only in-tree user of pskb_extract() is RDS, which copies the queued data
to userspace and never calls a checksum validation helper, so there is no
functional failure visible today; the concern is the trust bits left on a
generic exported helper.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915130423.3956471-1-edumazet%40google.com

Re: [PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[Paolo Abeni]

On 9/16/26 15:05, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] Incomplete invalidation of checksum metadata in the new
>    skb_carve_reset_headers() (net/core/skbuff.c:6842).
> 
> --
> 
> net: skbuff: do not leave stale header offsets after pskb_carve()
> 
> The two pskb_carve() helpers used to call skb_headers_offset_update(skb, 0),
> which left mac/network/transport offsets describing bytes that were just
> removed. This patch adds skb_carve_reset_headers() to unset the mac and
> transport headers, reset the network header, clear mac_len, zero the inner
> header state, and downgrade CHECKSUM_PARTIAL to CHECKSUM_NONE.
> 
> Closes: https://lore.kernel.org/netdev/6aa3e9d3.f2639fcc.29487d.0028.GAE@google.com/
> 
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index cc3b4b70288b4..609f2c7f4a47a 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -6832,6 +6832,34 @@ struct sk_buff *alloc_skb_with_frags(unsigned long header_len,
>>   }
>>   EXPORT_SYMBOL(alloc_skb_with_frags);
>>   
>> +/* pskb_carve_inside_header() and pskb_carve_inside_nonlinear()
>> + * remove the first bytes of a packet and reallocate skb->head.
>> + *
>> + * Whatever headers were present before the operation are gone,
>> + * we must not leave stale offsets, otherwise users of this skb
>> + * (skb_dump(), drop_monitor, taps, ...) would read or pull garbage.
>> + */
>> +static void skb_carve_reset_headers(struct sk_buff *skb)
>> +{
>> +	skb_unset_mac_header(skb);
>> +	skb_unset_transport_header(skb);
>> +	skb_reset_network_header(skb);
>> +	skb->mac_len = 0;
>> +
>> +	/* Inner offsets have no "unset" marker, zero them so that
>> +	 * skb_inner_network_header_was_set() becomes false and no
>> +	 * consumer mistakes them for a real (and long gone) header.
>> +	 */
>> +	skb->inner_mac_header = 0;
>> +	skb->inner_network_header = 0;
>> +	skb->inner_transport_header = 0;
>> +	skb->inner_protocol = 0;
>> +	skb->encapsulation = 0;
>> +
>> +	if (skb->ip_summed == CHECKSUM_PARTIAL)
>> +		skb->ip_summed = CHECKSUM_NONE;
>> +}
> 
> [Severity: Medium]
> The changelog says the goal is that "all the header state is invalidated in
> one place".  Is the checksum state fully invalidated here?
> 
> skb_carve_reset_headers() only rewrites ip_summed, while skb->csum,
> skb->csum_valid, skb->csum_complete_sw, skb->csum_level and
> skb->csum_not_inet keep the values they had before the bytes were removed.
> 
> For the CHECKSUM_PARTIAL case, the syzbot dump quoted in the changelog shows
> the incoming skb already has valid=1:
> 
>    csum(0x100120 start=288 offset=16 ip_summed=3 complete_sw=0 valid=1 level=0)
> 
> After the downgrade this becomes ip_summed=CHECKSUM_NONE with csum_valid
> still 1, and skb_csum_unnecessary() in include/linux/skbuff.h returns true on
> csum_valid alone:
> 
>    static inline int skb_csum_unnecessary(const struct sk_buff *skb)
>    {
> 	return ((skb->ip_summed == CHECKSUM_UNNECESSARY) ||
> 		skb->csum_valid ||
> 		(skb->ip_summed == CHECKSUM_PARTIAL &&
> 		 skb_checksum_start_offset(skb) >= 0));
>    }
> 
> Can a consumer of the carved skb then believe the payload was already
> verified, even though the bytes the checksum covered are gone?  A leftover
> csum_not_inet would similarly still claim CRC32c semantics, and skb_dump()
> would print the contradictory "ip_summed=0 ... valid=1".
> 
> Should csum_valid, csum_complete_sw, csum_level and csum_not_inet be cleared
> in the same helper?
> 
> The CHECKSUM_COMPLETE case looks untouched as well.  Unlike
> skb_pull_rcsum()/skb_postpull_rcsum(), the carve path never subtracts the off
> bytes removed from the front from skb->csum, and pskb_extract() trims the tail
> with plain pskb_trim() rather than pskb_trim_rcsum():
> 
>    net/core/skbuff.c:pskb_extract() {
> 	...
> 	if (pskb_carve(clone, off, gfp) < 0 ||
> 	    pskb_trim(clone, to_copy)) {
> 	...
>    }
> 
> So a CHECKSUM_COMPLETE skb keeps a whole-packet checksum that no longer
> matches its contents.  Would it make sense to also drop or fix up
> CHECKSUM_COMPLETE in skb_carve_reset_headers(), or to use pskb_trim_rcsum()
> in pskb_extract()?
> 
> The only in-tree user of pskb_extract() is RDS, which copies the queued data
> to userspace and never calls a checksum validation helper, so there is no
> functional failure visible today; the concern is the trust bits left on a
> generic exported helper.

Given the last paragraph all the above is IMHO material for eventual future
cleanups.

/P

Re: [PATCH v2 net] net: skbuff: do not leave stale header offsets after pskb_carve()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 15 Sep 2026 13:04:23 +0000 you wrote:
> pskb_carve_inside_header() and pskb_carve_inside_nonlinear() remove
> the first bytes of a packet and reallocate skb->head.
> 
> All the headers that were present before the operation are gone,
> but both functions call skb_headers_offset_update(skb, 0), which
> is a no-op : skb->mac_header, skb->network_header,
> skb->transport_header and skb->csum_start keep their old values and
> now describe bytes which are no longer there.
> 
> [...]

Here is the summary with links:
  - [v2,net] net: skbuff: do not leave stale header offsets after pskb_carve()
    https://git.kernel.org/netdev/net/c/a5117e1eccac

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html