[PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path

[PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path

[David Carlier]

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)) {
 				if (is_ifindex_excluded(excluded_devices, num_excluded,
 							dst->dev->ifindex))
 					continue;
-- 
2.53.0

Re: [PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path

[Jiayuan Chen]

On 3/19/26 12: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>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Re: [PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path

[Martin KaFai Lau]

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.

>   				if (is_ifindex_excluded(excluded_devices, num_excluded,
>   							dst->dev->ifindex))
>   					continue;

Re: [PATCH] bpf: use RCU-safe iteration in dev_map_redirect_multi() SKB path

[Martin KaFai Lau]

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;
> 
>