Skip to content

Commit 2fb541c

Browse files
Yunsheng Lindavem330
authored andcommitted
net: sch_generic: aviod concurrent reset and enqueue op for lockless qdisc
Currently there is concurrent reset and enqueue operation for the same lockless qdisc when there is no lock to synchronize the q->enqueue() in __dev_xmit_skb() with the qdisc reset operation in qdisc_deactivate() called by dev_deactivate_queue(), which may cause out-of-bounds access for priv->ring[] in hns3 driver if user has requested a smaller queue num when __dev_xmit_skb() still enqueue a skb with a larger queue_mapping after the corresponding qdisc is reset, and call hns3_nic_net_xmit() with that skb later. Reused the existing synchronize_net() in dev_deactivate_many() to make sure skb with larger queue_mapping enqueued to old qdisc(which is saved in dev_queue->qdisc_sleeping) will always be reset when dev_reset_queue() is called. Fixes: 6b3ba91 ("net: sched: allow qdiscs to handle locking") Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent edecfa9 commit 2fb541c

File tree

1 file changed

+33
-15
lines changed

1 file changed

+33
-15
lines changed

net/sched/sch_generic.c

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,24 +1131,10 @@ EXPORT_SYMBOL(dev_activate);
11311131

11321132
static void qdisc_deactivate(struct Qdisc *qdisc)
11331133
{
1134-
bool nolock = qdisc->flags & TCQ_F_NOLOCK;
1135-
11361134
if (qdisc->flags & TCQ_F_BUILTIN)
11371135
return;
1138-
if (test_bit(__QDISC_STATE_DEACTIVATED, &qdisc->state))
1139-
return;
1140-
1141-
if (nolock)
1142-
spin_lock_bh(&qdisc->seqlock);
1143-
spin_lock_bh(qdisc_lock(qdisc));
11441136

11451137
set_bit(__QDISC_STATE_DEACTIVATED, &qdisc->state);
1146-
1147-
qdisc_reset(qdisc);
1148-
1149-
spin_unlock_bh(qdisc_lock(qdisc));
1150-
if (nolock)
1151-
spin_unlock_bh(&qdisc->seqlock);
11521138
}
11531139

11541140
static void dev_deactivate_queue(struct net_device *dev,
@@ -1165,6 +1151,30 @@ static void dev_deactivate_queue(struct net_device *dev,
11651151
}
11661152
}
11671153

1154+
static void dev_reset_queue(struct net_device *dev,
1155+
struct netdev_queue *dev_queue,
1156+
void *_unused)
1157+
{
1158+
struct Qdisc *qdisc;
1159+
bool nolock;
1160+
1161+
qdisc = dev_queue->qdisc_sleeping;
1162+
if (!qdisc)
1163+
return;
1164+
1165+
nolock = qdisc->flags & TCQ_F_NOLOCK;
1166+
1167+
if (nolock)
1168+
spin_lock_bh(&qdisc->seqlock);
1169+
spin_lock_bh(qdisc_lock(qdisc));
1170+
1171+
qdisc_reset(qdisc);
1172+
1173+
spin_unlock_bh(qdisc_lock(qdisc));
1174+
if (nolock)
1175+
spin_unlock_bh(&qdisc->seqlock);
1176+
}
1177+
11681178
static bool some_qdisc_is_busy(struct net_device *dev)
11691179
{
11701180
unsigned int i;
@@ -1213,12 +1223,20 @@ void dev_deactivate_many(struct list_head *head)
12131223
dev_watchdog_down(dev);
12141224
}
12151225

1216-
/* Wait for outstanding qdisc-less dev_queue_xmit calls.
1226+
/* Wait for outstanding qdisc-less dev_queue_xmit calls or
1227+
* outstanding qdisc enqueuing calls.
12171228
* This is avoided if all devices are in dismantle phase :
12181229
* Caller will call synchronize_net() for us
12191230
*/
12201231
synchronize_net();
12211232

1233+
list_for_each_entry(dev, head, close_list) {
1234+
netdev_for_each_tx_queue(dev, dev_reset_queue, NULL);
1235+
1236+
if (dev_ingress_queue(dev))
1237+
dev_reset_queue(dev, dev_ingress_queue(dev), NULL);
1238+
}
1239+
12221240
/* Wait for outstanding qdisc_run calls. */
12231241
list_for_each_entry(dev, head, close_list) {
12241242
while (some_qdisc_is_busy(dev)) {

0 commit comments

Comments
 (0)