github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-11-07 06:17:02 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

net/sched: sch_teql: move rcu_read_lock()/spin_lock() from _bh variants

Browse Source 
This is a followup based on sashiko comments [1] on commit e5b811fe79
("net/sched: sch_teql: Introduce slaves_lock to avoid race condition and UAF")

Use plain rcu_read_lock()/spin_lock() in teql_master_xmit() instead of the
_bh variants, since ndo_start_xmit is already invoked with BH disabled
by the core stack and the _bh primitives can warn in_hardirq() when xmit
is reached through netpoll or a softirq xmit path with hard IRQs disabled.

Moves rcu_read_lock() after restart: label + adds rcu_read_unlock() before
goto restart (fixes the unbounded RCU hold across retries)

[1] https://sashiko.dev/#/patchset/20260628111229.669751-1-jhs%40mojatatu.com

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Fixes: e5b811fe79 ("net/sched: sch_teql: Introduce slaves_lock to avoid race condition and UAF")
Link: https://patch.msgid.link/20260630150922.238714-1-jhs@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Jamal Hadi Salim 2026-06-30 11:09:22 -04:00 committed by Paolo Abeni
parent d335dcc6f5
commit 6301f6a34e
1 changed files with 14 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
net/sched/sch_teql.c
View File

@ -311,14 +311,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
int subq = skb_get_queue_mapping(skb); int subq = skb_get_queue_mapping(skb);
struct sk_buff *skb_res = NULL; struct sk_buff *skb_res = NULL;
rcu_read_lock_bh();
start = rcu_dereference_bh(master->slaves);
restart: restart:
nores = 0; nores = 0;
busy = 0; busy = 0;
rcu_read_lock();
start = rcu_dereference(master->slaves);
q = start; q = start;
if (!q) if (!q)
goto drop; goto drop;
@ -345,17 +345,17 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
netdev_start_xmit(skb, slave, slave_txq, false) == netdev_start_xmit(skb, slave, slave_txq, false) ==
NETDEV_TX_OK) { NETDEV_TX_OK) {
__netif_tx_unlock(slave_txq); __netif_tx_unlock(slave_txq);
spin_lock_bh(&master->slaves_lock); spin_lock(&master->slaves_lock);
if (rcu_dereference_protected(master->slaves, if (rcu_dereference_protected(master->slaves,
lockdep_is_held(&master->slaves_lock)) == q) lockdep_is_held(&master->slaves_lock)) == q)
rcu_assign_pointer(master->slaves, rcu_assign_pointer(master->slaves,
rcu_dereference_protected(NEXT_SLAVE(q), rcu_dereference_protected(NEXT_SLAVE(q),
lockdep_is_held(&master->slaves_lock))); lockdep_is_held(&master->slaves_lock)));
spin_unlock_bh(&master->slaves_lock); spin_unlock(&master->slaves_lock);
netif_wake_queue(dev); netif_wake_queue(dev);
master->tx_packets++; master->tx_packets++;
master->tx_bytes += length; master->tx_bytes += length;
rcu_read_unlock_bh(); rcu_read_unlock();
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }
__netif_tx_unlock(slave_txq); __netif_tx_unlock(slave_txq);
@ -364,37 +364,38 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
busy = 1; busy = 1;
break; break;
case 1: case 1:
spin_lock_bh(&master->slaves_lock); spin_lock(&master->slaves_lock);
if (rcu_dereference_protected(master->slaves, if (rcu_dereference_protected(master->slaves,
lockdep_is_held(&master->slaves_lock)) == q) lockdep_is_held(&master->slaves_lock)) == q)
rcu_assign_pointer(master->slaves, rcu_assign_pointer(master->slaves,
rcu_dereference_protected(NEXT_SLAVE(q), rcu_dereference_protected(NEXT_SLAVE(q),
lockdep_is_held(&master->slaves_lock))); lockdep_is_held(&master->slaves_lock)));
spin_unlock_bh(&master->slaves_lock); spin_unlock(&master->slaves_lock);
rcu_read_unlock_bh(); rcu_read_unlock();
return NETDEV_TX_OK; return NETDEV_TX_OK;
default: default:
nores = 1; nores = 1;
break; break;
} }
__skb_pull(skb, skb_network_offset(skb)); __skb_pull(skb, skb_network_offset(skb));
} while ((q = rcu_dereference_bh(NEXT_SLAVE(q))) != start); } while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
if (nores && skb_res == NULL) { if (nores && skb_res == NULL) {
skb_res = skb; skb_res = skb;
rcu_read_unlock();
goto restart; goto restart;
} }
if (busy) { if (busy) {
netif_stop_queue(dev); netif_stop_queue(dev);
rcu_read_unlock_bh(); rcu_read_unlock();
return NETDEV_TX_BUSY; return NETDEV_TX_BUSY;
} }
master->tx_errors++; master->tx_errors++;
drop: drop:
master->tx_dropped++; master->tx_dropped++;
rcu_read_unlock_bh(); rcu_read_unlock();
dev_kfree_skb(skb); dev_kfree_skb(skb);
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }