On Sunday, 17 May 2026 18:38:53 CEST Linus Lüssing wrote: > On Thu, May 14, 2026 at 07:41:38PM +0200, Sven Eckelmann wrote: > > batadv_mcast_purge_orig() removes entries from RCU-protected hlists but > > does not wait for an RCU grace period before returning. Concurrent RCU > > readers may still accesses references to those entries at the point of > > removal. RCU-protected readers trying to operate on entries like > > orig->mcast_want_all_ipv6_node will then access already freed memory. > > This one I don't really get yet. The mcat_want_all_* lists/entries should > be spinlock protected (&bat_priv->mcast.want_lists_lock), not RCU > protected? > > We don't use RCU for these lists in the first place because within > the list changes / spinlocks &bat_priv->mcast.num_want_all_* > atomic counters are increased/decreased. And these atomic counters > are then used in fast path. Not those lists. >
Um? I can see RCU modification function here (which are correctly protected
by spinlocks):
static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
struct batadv_orig_node *orig,
u8 mcast_flags)
{
struct hlist_node *node = &orig->mcast_want_all_ipv4_node;
struct hlist_head *head = &bat_priv->mcast.want_all_ipv4_list;
lockdep_assert_held(&orig->mcast_handler_lock);
/* switched from flag unset to set */
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV4 &&
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4)) {
[...]
hlist_add_head_rcu(node, head);
[...]
/* switched from flag set to unset */
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) &&
orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) {
[...]
hlist_del_init_rcu(node);
[...]
}
}
But this looks super RCU-like (without locks):
static int
batadv_mcast_forw_want_all_ipv4(struct batadv_priv *bat_priv,
struct sk_buff *skb, unsigned short vid)
{
struct batadv_orig_node *orig_node;
int ret = NET_XMIT_SUCCESS;
struct sk_buff *newskb;
rcu_read_lock();
hlist_for_each_entry_rcu(orig_node,
&bat_priv->mcast.want_all_ipv4_list,
mcast_want_all_ipv4_node) {
[..]
}
rcu_read_unlock();
return ret;
}
And when you do something like this, you can't try do run these functions in a
free_rcu function. Because you are then missing the RCU grace period. The list
can still be accessed in a parallel running RCU reader and the
batadv_orig_node_free_rcu function might then already have freed the originator.
The reader then goes *KABUMM*.
Or am I missing something and the functions themelf need to be freed from RCU
references (or something else)?
Regards,
Sven
signature.asc
Description: This is a digitally signed message part.
