<!DOCTYPE html>
<!-- BaNnErBlUrFlE-BoDy-start -->
<!-- Preheader Text : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;">
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</div>
<!-- Preheader Text : END -->
<!-- Email Banner : BEGIN -->
<div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;max-height:0px;opacity:0;overflow:hidden;"></div>
<!-- Email Banner : END -->
<!-- BaNnErBlUrFlE-BoDy-end -->
<html>
<head><!-- BaNnErBlUrFlE-HeAdEr-start -->
<style>
#pfptBanner4x1t8y5 { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important;
background-color: #c2d4d4 !important;
max-width: none !important; max-height: none !important }
.pfptPrimaryButton4x1t8y5:hover, .pfptPrimaryButton4x1t8y5:focus {
background-color: #a2b1b1 !important; }
.pfptPrimaryButton4x1t8y5:active {
background-color: #828e8e !important; }
html:root, html:root>body { all: revert !important; display: block !important;
visibility: visible !important; opacity: 1 !important; }
</style>
<!-- BaNnErBlUrFlE-HeAdEr-end -->
</head><body><pre style="font-family: sans-serif; font-size: 100%; white-space: pre-wrap; word-wrap: break-word">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
</pre></body></html>