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

This is a followup based on sashiko comments [1] on commit e5b811fe7931
("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: e5b811fe7931 ("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>

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 24ba31f8c82855d9f7bb8c1a5f1e94b91f7f6371..5c42a29a981cb623de6500493a488b6450200b1b 100644 (file)
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -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 @@ restart:
                                    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 @@ restart:
                                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;
 }