netdev - Re: [PATCH net-next 2/3] net: neigh: use kfree_skb_reason() for __neigh_event

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Feb 2022 11:47:38 +0800
From:   Menglong Dong <menglong8.dong@...il.com>
To:     David Ahern <dsahern@...nel.org>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        David Miller <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Menglong Dong <imagedong@...cent.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Alexander Lobakin <alobakin@...me>,
        Cong Wang <cong.wang@...edance.com>, paulb@...dia.com,
        Talal Ahmad <talalahmad@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        flyingpeng@...cent.com, Mengen Sun <mengensun@...cent.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Yajun Deng <yajun.deng@...ux.dev>,
        Roopa Prabhu <roopa@...dia.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/3] net: neigh: use kfree_skb_reason() for __neigh_event_send()

On Tue, Feb 22, 2022 at 11:17 AM David Ahern <dsahern@...nel.org> wrote:
>
> On 2/20/22 8:57 AM, menglong8.dong@...il.com wrote:
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index c310a4a8fc86..206b66f5ce6b 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -393,6 +393,15 @@ enum skb_drop_reason {
> >                                        * see the doc for disable_ipv6
> >                                        * in ip-sysctl.rst for detail
> >                                        */
> > +     SKB_DROP_REASON_NEIGH_FAILED,   /* dropped as the state of
> > +                                      * neighbour is NUD_FAILED
> > +                                      */
>
> /* neigh entry in failed state */
>
> > +     SKB_DROP_REASON_NEIGH_QUEUEFULL,        /* the skbs that waiting
> > +                                              * for sending on the queue
> > +                                              * of neigh->arp_queue is
> > +                                              * full, and the skbs on the
> > +                                              * tail will be dropped
> > +                                              */
>
> /* arp_queue for neigh entry is full */
>
>
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index ec0bf737b076..c353834e8fa9 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -1171,7 +1171,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb,
> >                       neigh->updated = jiffies;
> >                       write_unlock_bh(&neigh->lock);
> >
> > -                     kfree_skb(skb);
> > +                     kfree_skb_reason(skb, SKB_DROP_REASON_NEIGH_FAILED);
> >                       return 1;
> >               }
> >       } else if (neigh->nud_state & NUD_STALE) {
> > @@ -1193,7 +1193,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb,
> >                               if (!buff)
> >                                       break;
> >                               neigh->arp_queue_len_bytes -= buff->truesize;
> > -                             kfree_skb(buff);
> > +                             kfree_skb_reason(buff, SKB_DROP_REASON_NEIGH_QUEUEFULL);
> >                               NEIGH_CACHE_STAT_INC(neigh->tbl, unres_discards);
> >                       }
> >                       skb_dst_force(skb);
>
> what about out_dead: path? the tracepoint there shows that path is of
> interest.

You are right, that path should be considered too.