* Re: [PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path
[not found] ` <27a6be7f-8263-41ef-a9b6-3925ee59061c@linux.dev>
@ 2026-03-20 1:59 ` Martin KaFai Lau
0 siblings, 0 replies; only message in thread
From: Martin KaFai Lau @ 2026-03-20 1:59 UTC (permalink / raw)
To: David Carlier
Cc: Alexei Starovoitov, Daniel Borkmann, davem, Jakub Kicinski,
netdev, bpf
On 3/19/26 6:37 PM, Martin KaFai Lau wrote:
>
>
> On 3/18/26 9:24 AM, David Carlier wrote:
>> The DEVMAP_HASH branch in dev_map_redirect_multi() uses
>> hlist_for_each_entry_safe() to iterate hash buckets, but this function
>> runs under RCU protection (called from xdp_do_generic_redirect_map()
>> in softirq context). Concurrent writers (__dev_map_hash_update_elem,
>> dev_map_hash_delete_elem) modify the list using RCU primitives
>> (hlist_add_head_rcu, hlist_del_rcu).
>>
>> hlist_for_each_entry_safe() performs plain pointer dereferences without
>> rcu_dereference(), missing the acquire barrier needed to pair with
>> writers' rcu_assign_pointer(). On weakly-ordered architectures (ARM64,
>> POWER), a reader can observe a partially-constructed node. It also
>> defeats CONFIG_PROVE_RCU lockdep validation and KCSAN data-race
>> detection.
>>
>> Replace with hlist_for_each_entry_rcu(), matching the XDP-frame path
>> (dev_map_enqueue_multi) which already uses the correct macro for the
>> same hash iteration.
>
>>
>> Fixes: e624d4ed4aa8 ("xdp: Extend xdp_redirect_map with broadcast
>> support")
>> Signed-off-by: David Carlier <devnexen@gmail.com>
>> ---
>> kernel/bpf/devmap.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
>> index 3d619d01088e..c8d256405c29 100644
>> --- a/kernel/bpf/devmap.c
>> +++ b/kernel/bpf/devmap.c
>> @@ -747,7 +747,6 @@ int dev_map_redirect_multi(struct net_device *dev,
>> struct sk_buff *skb,
>> struct bpf_dtab_netdev *dst, *last_dst = NULL;
>> int excluded_devices[1+MAX_NEST_DEV];
>> struct hlist_head *head;
>> - struct hlist_node *next;
>> int num_excluded = 0;
>> unsigned int i;
>> int err;
>> @@ -787,7 +786,7 @@ int dev_map_redirect_multi(struct net_device *dev,
>> struct sk_buff *skb,
>> } else { /* BPF_MAP_TYPE_DEVMAP_HASH */
>> for (i = 0; i < dtab->n_buckets; i++) {
>> head = dev_map_index_hash(dtab, i);
>> - hlist_for_each_entry_safe(dst, next, head, index_hlist) {
>> + hlist_for_each_entry_rcu(dst, head, index_hlist,
>> lockdep_is_held(&dtab->index_lock)) {he
>
> Where is the dtab->index_lock acquired? dev_map_enqueue_multi() has been
> incorrect also. Take a look at the rcu_read_lock_bh_held() usage in the
> rcu_dereference_check() a few lines above.
>
> pw-bot: cr
>
> Please cc the bpf list and tag the target tree in the subject. imo, bpf-
> next instead of bpf should be fine for this.
cc bpf list.
>
>> if (is_ifindex_excluded(excluded_devices, num_excluded,
>> dst->dev->ifindex))
>> continue;
>
>
^ permalink raw reply [flat|nested] only message in thread