netdev - Re: [PATCH net] net: ipv4: fix stat increase when udp early demux drops the packet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a2snjvff2xus2fp235cwaz5q7vdbbcyr7xokt6axbahsajn7a6@4b3b24ltf3ec>
Date: Wed, 25 Jun 2025 12:32:29 +0200
From: Antoine Tenart <atenart@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Antoine Tenart <atenart@...nel.org>, davem@...emloft.net, 
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, 
	Menglong Dong <menglong8.dong@...il.com>, Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [PATCH net] net: ipv4: fix stat increase when udp early demux
 drops the packet

On Wed, Jun 25, 2025 at 02:43:22AM -0700, Eric Dumazet wrote:
> On Wed, Jun 25, 2025 at 2:00 AM Antoine Tenart <atenart@...nel.org> wrote:
> > @@ -319,8 +319,8 @@ static int ip_rcv_finish_core(struct net *net,
> >                               const struct sk_buff *hint)
> >  {
> >         const struct iphdr *iph = ip_hdr(skb);
> > -       int err, drop_reason;
> >         struct rtable *rt;
> > +       int drop_reason;
> >
> >         if (ip_can_use_hint(skb, iph, hint)) {
> >                 drop_reason = ip_route_use_hint(skb, iph->daddr, iph->saddr,
> > @@ -345,8 +345,8 @@ static int ip_rcv_finish_core(struct net *net,
> >                         break;
> >                 case IPPROTO_UDP:
> >                         if (READ_ONCE(net->ipv4.sysctl_udp_early_demux)) {
> > -                               err = udp_v4_early_demux(skb);
> > -                               if (unlikely(err))
> > +                               drop_reason = udp_v4_early_demux(skb);
> > +                               if (unlikely(drop_reason))
> >                                         goto drop_error;
> 
> 
> This would leave @drop_reason == SKB_NOT_DROPPED_YET here, furtur
> "goto drop;" would be confused.
> 
> if (iph->ihl > 5 && ip_rcv_options(skb, dev))
>      goto drop;    // Oops

Good catch, thanks!

> The following would be easier to review ?
> 
> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index 30a5e9460d006de306b5bac49c92f9b9bf21f2f5..d105df76dd81696d98b3df581934c27af2e0494e
> 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -346,8 +346,10 @@ static int ip_rcv_finish_core(struct net *net,
>                 case IPPROTO_UDP:
>                         if (READ_ONCE(net->ipv4.sysctl_udp_early_demux)) {
>                                 err = udp_v4_early_demux(skb);
> -                               if (unlikely(err))
> +                               if (unlikely(err)) {
> +                                       drop_reason = err; /* temporary fix. */
>                                         goto drop_error;
> +                               }
> 
>                                 /* must reload iph, skb->head might
> have changed */
>                                 iph = ip_hdr(skb);
> 

Or just resetting the drop reason to SKB_DROP_REASON_NOT_SPECIFIED, to
keep it consistent with the rest of the function?

As a follow-up we could remove those and set the reason explicitly
before the drop to avoid such issue later. That makes things easier to
read too.

Thanks,
Antoine