netdev - Re: [PATCH net-next 1/8] net: core: neighbour: Add a neigh_fill

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]

Message-ID: <CAAVpQUAoo0JBCPgf_Mc4cO2tpUmn0=Rn7aiUj0Q7HiHWRyoWpA@mail.gmail.com>
Date: Thu, 15 Jan 2026 23:22:38 -0800
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Petr Machata <petrm@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	netdev@...r.kernel.org, Ido Schimmel <idosch@...dia.com>, 
	Breno Leitao <leitao@...ian.org>, Andy Roulin <aroulin@...dia.com>, 
	Francesco Ruggeri <fruggeri@...sta.com>, Stephen Hemminger <stephen@...workplumber.org>, mlxsw@...dia.com
Subject: Re: [PATCH net-next 1/8] net: core: neighbour: Add a
 neigh_fill_info() helper for when lock not held

On Wed, Jan 14, 2026 at 1:56 AM Petr Machata <petrm@...dia.com> wrote:
>
> The netlink message needs to be formatted and sent inside the critical
> section where the neighbor is changed, so that it reflects the
> notified-upon neighbor state. Because it will happen inside an already
> existing critical section, it has to assume that the neighbor lock is held.
> Add a helper __neigh_fill_info(), which is like neigh_fill_info(), but
> makes this assumption. Convert neigh_fill_info() to a wrapper around this
> new helper.
>
> Signed-off-by: Petr Machata <petrm@...dia.com>
> Reviewed-by: Ido Schimmel <idosch@...dia.com>
> ---
>  net/core/neighbour.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 96a3b1a93252..6cdd93dfa3ea 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2622,8 +2622,8 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
>         return skb->len;
>  }
>
> -static int neigh_fill_info(struct sk_buff *skb, struct neighbour *neigh,
> -                          u32 pid, u32 seq, int type, unsigned int flags)
> +static int __neigh_fill_info(struct sk_buff *skb, struct neighbour *neigh,
> +                            u32 pid, u32 seq, int type, unsigned int flags)
>  {
>         u32 neigh_flags, neigh_flags_ext;
>         unsigned long now = jiffies;
> @@ -2649,23 +2649,19 @@ static int neigh_fill_info(struct sk_buff *skb, struct neighbour *neigh,
>         if (nla_put(skb, NDA_DST, neigh->tbl->key_len, neigh->primary_key))
>                 goto nla_put_failure;
>
> -       read_lock_bh(&neigh->lock);
>         ndm->ndm_state   = neigh->nud_state;
>         if (neigh->nud_state & NUD_VALID) {
>                 char haddr[MAX_ADDR_LEN];
>
>                 neigh_ha_snapshot(haddr, neigh, neigh->dev);
> -               if (nla_put(skb, NDA_LLADDR, neigh->dev->addr_len, haddr) < 0) {
> -                       read_unlock_bh(&neigh->lock);
> +               if (nla_put(skb, NDA_LLADDR, neigh->dev->addr_len, haddr) < 0)
>                         goto nla_put_failure;
> -               }
>         }
>
>         ci.ndm_used      = jiffies_to_clock_t(now - neigh->used);
>         ci.ndm_confirmed = jiffies_to_clock_t(now - neigh->confirmed);
>         ci.ndm_updated   = jiffies_to_clock_t(now - neigh->updated);
>         ci.ndm_refcnt    = refcount_read(&neigh->refcnt) - 1;
> -       read_unlock_bh(&neigh->lock);
>
>         if (nla_put_u32(skb, NDA_PROBES, atomic_read(&neigh->probes)) ||
>             nla_put(skb, NDA_CACHEINFO, sizeof(ci), &ci))
> @@ -2684,6 +2680,20 @@ static int neigh_fill_info(struct sk_buff *skb, struct neighbour *neigh,
>         return -EMSGSIZE;
>  }
>
> +static int neigh_fill_info(struct sk_buff *skb, struct neighbour *neigh,
> +                          u32 pid, u32 seq, int type, unsigned int flags)
> +       __releases(neigh->lock)
> +       __acquires(neigh->lock)

nit: Does Sparse complain without these annotations even
a function has a paired lock/unlock ops ?


> +{
> +       int err;
> +
> +       read_lock_bh(&neigh->lock);
> +       err = __neigh_fill_info(skb, neigh, pid, seq, type, flags);
> +       read_unlock_bh(&neigh->lock);
> +
> +       return err;
> +}
> +
>  static int pneigh_fill_info(struct sk_buff *skb, struct pneigh_entry *pn,
>                             u32 pid, u32 seq, int type, unsigned int flags,
>                             struct neigh_table *tbl)
> --
> 2.51.1
>