[v2,net-next,07/15] udp: Remove partial csum code in RX.

Kuniyuki Iwashima kuniyu at google.com
Tue Mar 10 16:17:01 UTC 2026


On Tue, Mar 10, 2026 at 3:26 AM Paolo Abeni <pabeni at redhat.com> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> udp: Remove partial csum code in RX.
>
> This commit removes UDP-Lite specific partial checksum handling from UDP
> receive paths, including removal of UDP_SKB_CB fields, simplification of
> checksum initialization functions, and elimination of UDP-Lite guards.
>
> Link: https://lore.kernel.org/netdev/20260305215013.2984628-8-kuniyu@google.com/
>
> > diff --git a/include/net/udp.h b/include/net/udp.h
> > --- a/include/net/udp.h
> > +++ b/include/net/udp.h
>
> [ ... ]
>
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
>
> [ ... ]
>
> > @@ -2703,14 +2647,15 @@ static int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
> >       if (ulen > skb->len)
> >               goto short_packet;
> >
> > -     if (proto == IPPROTO_UDP) {
> > -             /* UDP validates ulen. */
> > -             if (ulen < sizeof(*uh) || pskb_trim_rcsum(skb, ulen))
> > +     /* UDP validates ulen. */
> > +     if (ulen < sizeof(*uh)) {
> > +             if (pskb_trim_rcsum(skb, ulen))
> >                       goto short_packet;
> > +
> >               uh = udp_hdr(skb);
> >       }
> >
> > -     if (udp4_csum_init(skb, uh, proto))
> > +     if (udp4_csum_init(skb, uh))
> >               goto csum_error;
> >
>
> Does this refactoring introduce two bugs?
>
> The old code checked both conditions with short-circuit OR:
>   if (ulen < sizeof(*uh) || pskb_trim_rcsum(skb, ulen))
>       goto short_packet;
>
> This correctly (1) rejected packets where ulen < 8, and (2) trimmed the
> SKB to the declared UDP length when ulen < skb->len.
>
> The new code changes the logic structure:
>   if (ulen < sizeof(*uh)) {
>       if (pskb_trim_rcsum(skb, ulen))
>           goto short_packet;
>       uh = udp_hdr(skb);
>   }
>
> First potential bug: When sizeof(*uh) <= ulen < skb->len, the entire block
> is skipped. This means pskb_trim_rcsum() is never called, so the SKB
> retains trailing data beyond the declared UDP length. Won't this cause
> checksum calculations to include extraneous bytes, leading to valid packets
> being incorrectly rejected?
>
> This scenario occurs when UDP length < IP payload length, which can happen
> with malformed or crafted packets.
>
> Second potential bug: When ulen < sizeof(*uh), the old code would
> immediately goto short_packet via the || short-circuit. The new code enters
> the block and tries to trim the SKB to a sub-header-size length. If
> pskb_trim_rcsum() succeeds, processing continues with skb->len <
> sizeof(struct udphdr). Is this safe?
>
> Looking at the IPv6 counterpart __udp6_lib_rcv(), it handles this
> differently:
>
>   if (ulen < sizeof(*uh))
>       goto short_packet;
>
>   if (ulen < skb->len) {
>       if (pskb_trim_rcsum(skb, ulen))
>           goto short_packet;
>       saddr = &ipv6_hdr(skb)->saddr;
>       daddr = &ipv6_hdr(skb)->daddr;
>       uh = udp_hdr(skb);
>   }
>
> Should the IPv4 code follow the same pattern, or should it preserve the
> original short-circuit logic:

Indeed, I'll follow the IPv6 pattern there.

Thanks !



More information about the Linux-security-module-archive mailing list