mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-07-22 02:17:36 -04:00
net/sched: sch_teql: move rcu_read_lock()/spin_lock() from _bh variants
This is a followup based on sashiko comments [1] on commite5b811fe79("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:
committed by
Paolo Abeni
parent
d335dcc6f5
commit
6301f6a34e
@@ -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);
|
||||
struct sk_buff *skb_res = NULL;
|
||||
|
||||
rcu_read_lock_bh();
|
||||
|
||||
start = rcu_dereference_bh(master->slaves);
|
||||
|
||||
restart:
|
||||
nores = 0;
|
||||
busy = 0;
|
||||
|
||||
rcu_read_lock();
|
||||
|
||||
start = rcu_dereference(master->slaves);
|
||||
|
||||
q = start;
|
||||
if (!q)
|
||||
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_TX_OK) {
|
||||
__netif_tx_unlock(slave_txq);
|
||||
spin_lock_bh(&master->slaves_lock);
|
||||
spin_lock(&master->slaves_lock);
|
||||
if (rcu_dereference_protected(master->slaves,
|
||||
lockdep_is_held(&master->slaves_lock)) == q)
|
||||
rcu_assign_pointer(master->slaves,
|
||||
rcu_dereference_protected(NEXT_SLAVE(q),
|
||||
lockdep_is_held(&master->slaves_lock)));
|
||||
spin_unlock_bh(&master->slaves_lock);
|
||||
spin_unlock(&master->slaves_lock);
|
||||
netif_wake_queue(dev);
|
||||
master->tx_packets++;
|
||||
master->tx_bytes += length;
|
||||
rcu_read_unlock_bh();
|
||||
rcu_read_unlock();
|
||||
return NETDEV_TX_OK;
|
||||
}
|
||||
__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;
|
||||
break;
|
||||
case 1:
|
||||
spin_lock_bh(&master->slaves_lock);
|
||||
spin_lock(&master->slaves_lock);
|
||||
if (rcu_dereference_protected(master->slaves,
|
||||
lockdep_is_held(&master->slaves_lock)) == q)
|
||||
rcu_assign_pointer(master->slaves,
|
||||
rcu_dereference_protected(NEXT_SLAVE(q),
|
||||
lockdep_is_held(&master->slaves_lock)));
|
||||
spin_unlock_bh(&master->slaves_lock);
|
||||
rcu_read_unlock_bh();
|
||||
spin_unlock(&master->slaves_lock);
|
||||
rcu_read_unlock();
|
||||
return NETDEV_TX_OK;
|
||||
default:
|
||||
nores = 1;
|
||||
break;
|
||||
}
|
||||
__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) {
|
||||
skb_res = skb;
|
||||
rcu_read_unlock();
|
||||
goto restart;
|
||||
}
|
||||
|
||||
if (busy) {
|
||||
netif_stop_queue(dev);
|
||||
rcu_read_unlock_bh();
|
||||
rcu_read_unlock();
|
||||
return NETDEV_TX_BUSY;
|
||||
}
|
||||
master->tx_errors++;
|
||||
|
||||
drop:
|
||||
master->tx_dropped++;
|
||||
rcu_read_unlock_bh();
|
||||
rcu_read_unlock();
|
||||
dev_kfree_skb(skb);
|
||||
return NETDEV_TX_OK;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user