netdev - Re: [PATCH v1 net-next 06/14] neighbour: Free pneigh

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: <20250712150159.GD721198@horms.kernel.org>
Date: Sat, 12 Jul 2025 16:01:59 +0100
From: Simon Horman <horms@...nel.org>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	David Ahern <dsahern@...nel.org>,
	Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v1 net-next 06/14] neighbour: Free pneigh_entry after RCU
 grace period.

On Fri, Jul 11, 2025 at 07:06:11PM +0000, Kuniyuki Iwashima wrote:
> We will convert RTM_GETNEIGH to RCU.
> 
> neigh_get() looks up pneigh_entry by pneigh_lookup() and passes
> it to pneigh_fill_info().
> 
> Then, we must ensure that the entry is alive till pneigh_fill_info()
> completes, but read_lock_bh(&tbl->lock) in pneigh_lookup() does not
> guarantee that.
> 
> Also, we will convert all readers of tbl->phash_buckets[] to RCU.
> 
> Let's use call_rcu() to free pneigh_entry and update phash_buckets[]
> and ->next by rcu_assign_pointer().
> 
> pneigh_ifdown_and_unlock() uses list_head to avoid overwriting
> ->next and moving RCU iterators to another list.
> 
> pndisc_destructor() (only IPv6 ndisc uses this) uses a mutex, so it
> is not delayed to call_rcu(), where we cannot sleep.  This is fine
> because the mcast code works with RCU and ipv6_dev_mc_dec() frees
> mcast objects after RCU grace period.
> 
> While at it, we change the return type of pneigh_ifdown_and_unlock()
> to void.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>
> ---
>  include/net/neighbour.h |  4 ++++
>  net/core/neighbour.c    | 51 +++++++++++++++++++++++++----------------
>  2 files changed, 35 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index 7f3d57da5689a..a877e56210b22 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -180,6 +180,10 @@ struct pneigh_entry {
>  	possible_net_t		net;
>  	struct net_device	*dev;
>  	netdevice_tracker	dev_tracker;
> +	union {
> +		struct list_head	free_node;
> +		struct rcu_head		rcu;
> +	};
>  	u32			flags;
>  	u8			protocol;
>  	bool			permanent;
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 814a45fb1962e..6725a40b2db3a 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -54,9 +54,9 @@ static void neigh_timer_handler(struct timer_list *t);
>  static void __neigh_notify(struct neighbour *n, int type, int flags,
>  			   u32 pid);
>  static void neigh_update_notify(struct neighbour *neigh, u32 nlmsg_pid);
> -static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
> -				    struct net_device *dev,
> -				    bool skip_perm);
> +static void pneigh_ifdown_and_unlock(struct neigh_table *tbl,
> +				     struct net_device *dev,
> +				     bool skip_perm);
>  
>  #ifdef CONFIG_PROC_FS
>  static const struct seq_operations neigh_stat_seq_ops;
> @@ -803,12 +803,20 @@ struct pneigh_entry *pneigh_create(struct neigh_table *tbl,
>  
>  	write_lock_bh(&tbl->lock);
>  	n->next = tbl->phash_buckets[hash_val];
> -	tbl->phash_buckets[hash_val] = n;
> +	rcu_assign_pointer(tbl->phash_buckets[hash_val], n);

Hi Iwashima-san,

A heads-up that unfortunately Sparse is unhappy about the __rcu annotations
here, and elsewhere in this patch (set).

For this patch I see:

  .../neighbour.c:860:33: error: incompatible types in comparison expression (different address spaces):
  .../neighbour.c:860:33:    struct pneigh_entry [noderef] __rcu *
  .../neighbour.c:860:33:    struct pneigh_entry *
  .../neighbour.c:806:9: error: incompatible types in comparison expression (different address spaces):
  .../neighbour.c:806:9:    struct pneigh_entry [noderef] __rcu *
  .../neighbour.c:806:9:    struct pneigh_entry *
  .../neighbour.c:832:25: error: incompatible types in comparison expression (different address spaces):
  .../neighbour.c:832:25:    struct pneigh_entry [noderef] __rcu *
  .../neighbour.c:832:25:    struct pneigh_entry *

...

-- 
pw-bot: changes-requested