Skip to content

Commit 7234e03

Browse files
davem330davem330
committed
Merge branch 'netdev_unregister_races'
Julian Anastasov says: ==================== net: fixes for device unregistration Test script from Eric W. Biederman can catch a problem where packets from backlog are processed long after the last synchronize_net call. This can be reproduced after few tests if commit 381c759 ("ipv4: Avoid crashing in ip_error") is reverted for the test. Incoming packets do not hold reference to device but even if they do, subsystems do not expect packets to fly during and after the NETDEV_UNREGISTER event. The first fix has the cost of netif_running check in fast path. The second fix calls rcu_read_lock while local IRQ is disabled, I hope this is not against the rules. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents a7d35f9 + 2c17d27 commit 7234e03

File tree

1 file changed

+20
-18
lines changed

1 file changed

+20
-18
lines changed

‎net/core/dev.c

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3448,6 +3448,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
34483448
local_irq_save(flags);
34493449

34503450
rps_lock(sd);
3451+
if (!netif_running(skb->dev))
3452+
goto drop;
34513453
qlen = skb_queue_len(&sd->input_pkt_queue);
34523454
if (qlen <= netdev_max_backlog && !skb_flow_limit(skb, qlen)) {
34533455
if (qlen) {
@@ -3469,6 +3471,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
34693471
goto enqueue;
34703472
}
34713473

3474+
drop:
34723475
sd->dropped++;
34733476
rps_unlock(sd);
34743477

@@ -3771,8 +3774,6 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
37713774

37723775
pt_prev = NULL;
37733776

3774-
rcu_read_lock();
3775-
37763777
another_round:
37773778
skb->skb_iif = skb->dev->ifindex;
37783779

@@ -3782,7 +3783,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
37823783
skb->protocol == cpu_to_be16(ETH_P_8021AD)) {
37833784
skb = skb_vlan_untag(skb);
37843785
if (unlikely(!skb))
3785-
goto unlock;
3786+
goto out;
37863787
}
37873788

37883789
#ifdef CONFIG_NET_CLS_ACT
@@ -3812,10 +3813,10 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
38123813
if (static_key_false(&ingress_needed)) {
38133814
skb = handle_ing(skb, &pt_prev, &ret, orig_dev);
38143815
if (!skb)
3815-
goto unlock;
3816+
goto out;
38163817

38173818
if (nf_ingress(skb, &pt_prev, &ret, orig_dev) < 0)
3818-
goto unlock;
3819+
goto out;
38193820
}
38203821
#endif
38213822
#ifdef CONFIG_NET_CLS_ACT
@@ -3833,7 +3834,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
38333834
if (vlan_do_receive(&skb))
38343835
goto another_round;
38353836
else if (unlikely(!skb))
3836-
goto unlock;
3837+
goto out;
38373838
}
38383839

38393840
rx_handler = rcu_dereference(skb->dev->rx_handler);
@@ -3845,7 +3846,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
38453846
switch (rx_handler(&skb)) {
38463847
case RX_HANDLER_CONSUMED:
38473848
ret = NET_RX_SUCCESS;
3848-
goto unlock;
3849+
goto out;
38493850
case RX_HANDLER_ANOTHER:
38503851
goto another_round;
38513852
case RX_HANDLER_EXACT:
@@ -3899,8 +3900,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
38993900
ret = NET_RX_DROP;
39003901
}
39013902

3902-
unlock:
3903-
rcu_read_unlock();
3903+
out:
39043904
return ret;
39053905
}
39063906

@@ -3931,29 +3931,30 @@ static int __netif_receive_skb(struct sk_buff *skb)
39313931

39323932
static int netif_receive_skb_internal(struct sk_buff *skb)
39333933
{
3934+
int ret;
3935+
39343936
net_timestamp_check(netdev_tstamp_prequeue, skb);
39353937

39363938
if (skb_defer_rx_timestamp(skb))
39373939
return NET_RX_SUCCESS;
39383940

3941+
rcu_read_lock();
3942+
39393943
#ifdef CONFIG_RPS
39403944
if (static_key_false(&rps_needed)) {
39413945
struct rps_dev_flow voidflow, *rflow = &voidflow;
3942-
int cpu, ret;
3943-
3944-
rcu_read_lock();
3945-
3946-
cpu = get_rps_cpu(skb->dev, skb, &rflow);
3946+
int cpu = get_rps_cpu(skb->dev, skb, &rflow);
39473947

39483948
if (cpu >= 0) {
39493949
ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
39503950
rcu_read_unlock();
39513951
return ret;
39523952
}
3953-
rcu_read_unlock();
39543953
}
39553954
#endif
3956-
return __netif_receive_skb(skb);
3955+
ret = __netif_receive_skb(skb);
3956+
rcu_read_unlock();
3957+
return ret;
39573958
}
39583959

39593960
/**
@@ -4498,8 +4499,10 @@ static int process_backlog(struct napi_struct *napi, int quota)
44984499
struct sk_buff *skb;
44994500

45004501
while ((skb = __skb_dequeue(&sd->process_queue))) {
4502+
rcu_read_lock();
45014503
local_irq_enable();
45024504
__netif_receive_skb(skb);
4505+
rcu_read_unlock();
45034506
local_irq_disable();
45044507
input_queue_head_incr(sd);
45054508
if (++work >= quota) {
@@ -6135,6 +6138,7 @@ static void rollback_registered_many(struct list_head *head)
61356138
unlist_netdevice(dev);
61366139

61376140
dev->reg_state = NETREG_UNREGISTERING;
6141+
on_each_cpu(flush_backlog, dev, 1);
61386142
}
61396143

61406144
synchronize_net();
@@ -6770,8 +6774,6 @@ void netdev_run_todo(void)
67706774

67716775
dev->reg_state = NETREG_UNREGISTERED;
67726776

6773-
on_each_cpu(flush_backlog, dev, 1);
6774-
67756777
netdev_wait_allrefs(dev);
67766778

67776779
/* paranoia */

0 commit comments

Comments
 (0)