Commit 7234e030 authored by David S. Miller's avatar David S. Miller

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 381c759d ("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: default avatarDavid S. Miller <davem@davemloft.net>
parents a7d35f9d 2c17d27c
...@@ -3448,6 +3448,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu, ...@@ -3448,6 +3448,8 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
local_irq_save(flags); local_irq_save(flags);
rps_lock(sd); rps_lock(sd);
if (!netif_running(skb->dev))
goto drop;
qlen = skb_queue_len(&sd->input_pkt_queue); qlen = skb_queue_len(&sd->input_pkt_queue);
if (qlen <= netdev_max_backlog && !skb_flow_limit(skb, qlen)) { if (qlen <= netdev_max_backlog && !skb_flow_limit(skb, qlen)) {
if (qlen) { if (qlen) {
...@@ -3469,6 +3471,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu, ...@@ -3469,6 +3471,7 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
goto enqueue; goto enqueue;
} }
drop:
sd->dropped++; sd->dropped++;
rps_unlock(sd); rps_unlock(sd);
...@@ -3771,8 +3774,6 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3771,8 +3774,6 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
pt_prev = NULL; pt_prev = NULL;
rcu_read_lock();
another_round: another_round:
skb->skb_iif = skb->dev->ifindex; skb->skb_iif = skb->dev->ifindex;
...@@ -3782,7 +3783,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3782,7 +3783,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
skb->protocol == cpu_to_be16(ETH_P_8021AD)) { skb->protocol == cpu_to_be16(ETH_P_8021AD)) {
skb = skb_vlan_untag(skb); skb = skb_vlan_untag(skb);
if (unlikely(!skb)) if (unlikely(!skb))
goto unlock; goto out;
} }
#ifdef CONFIG_NET_CLS_ACT #ifdef CONFIG_NET_CLS_ACT
...@@ -3812,10 +3813,10 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3812,10 +3813,10 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
if (static_key_false(&ingress_needed)) { if (static_key_false(&ingress_needed)) {
skb = handle_ing(skb, &pt_prev, &ret, orig_dev); skb = handle_ing(skb, &pt_prev, &ret, orig_dev);
if (!skb) if (!skb)
goto unlock; goto out;
if (nf_ingress(skb, &pt_prev, &ret, orig_dev) < 0) if (nf_ingress(skb, &pt_prev, &ret, orig_dev) < 0)
goto unlock; goto out;
} }
#endif #endif
#ifdef CONFIG_NET_CLS_ACT #ifdef CONFIG_NET_CLS_ACT
...@@ -3833,7 +3834,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3833,7 +3834,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
if (vlan_do_receive(&skb)) if (vlan_do_receive(&skb))
goto another_round; goto another_round;
else if (unlikely(!skb)) else if (unlikely(!skb))
goto unlock; goto out;
} }
rx_handler = rcu_dereference(skb->dev->rx_handler); rx_handler = rcu_dereference(skb->dev->rx_handler);
...@@ -3845,7 +3846,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3845,7 +3846,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
switch (rx_handler(&skb)) { switch (rx_handler(&skb)) {
case RX_HANDLER_CONSUMED: case RX_HANDLER_CONSUMED:
ret = NET_RX_SUCCESS; ret = NET_RX_SUCCESS;
goto unlock; goto out;
case RX_HANDLER_ANOTHER: case RX_HANDLER_ANOTHER:
goto another_round; goto another_round;
case RX_HANDLER_EXACT: case RX_HANDLER_EXACT:
...@@ -3899,8 +3900,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc) ...@@ -3899,8 +3900,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
ret = NET_RX_DROP; ret = NET_RX_DROP;
} }
unlock: out:
rcu_read_unlock();
return ret; return ret;
} }
...@@ -3931,29 +3931,30 @@ static int __netif_receive_skb(struct sk_buff *skb) ...@@ -3931,29 +3931,30 @@ static int __netif_receive_skb(struct sk_buff *skb)
static int netif_receive_skb_internal(struct sk_buff *skb) static int netif_receive_skb_internal(struct sk_buff *skb)
{ {
int ret;
net_timestamp_check(netdev_tstamp_prequeue, skb); net_timestamp_check(netdev_tstamp_prequeue, skb);
if (skb_defer_rx_timestamp(skb)) if (skb_defer_rx_timestamp(skb))
return NET_RX_SUCCESS; return NET_RX_SUCCESS;
rcu_read_lock();
#ifdef CONFIG_RPS #ifdef CONFIG_RPS
if (static_key_false(&rps_needed)) { if (static_key_false(&rps_needed)) {
struct rps_dev_flow voidflow, *rflow = &voidflow; struct rps_dev_flow voidflow, *rflow = &voidflow;
int cpu, ret; int cpu = get_rps_cpu(skb->dev, skb, &rflow);
rcu_read_lock();
cpu = get_rps_cpu(skb->dev, skb, &rflow);
if (cpu >= 0) { if (cpu >= 0) {
ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail); ret = enqueue_to_backlog(skb, cpu, &rflow->last_qtail);
rcu_read_unlock(); rcu_read_unlock();
return ret; return ret;
} }
rcu_read_unlock();
} }
#endif #endif
return __netif_receive_skb(skb); ret = __netif_receive_skb(skb);
rcu_read_unlock();
return ret;
} }
/** /**
...@@ -4498,8 +4499,10 @@ static int process_backlog(struct napi_struct *napi, int quota) ...@@ -4498,8 +4499,10 @@ static int process_backlog(struct napi_struct *napi, int quota)
struct sk_buff *skb; struct sk_buff *skb;
while ((skb = __skb_dequeue(&sd->process_queue))) { while ((skb = __skb_dequeue(&sd->process_queue))) {
rcu_read_lock();
local_irq_enable(); local_irq_enable();
__netif_receive_skb(skb); __netif_receive_skb(skb);
rcu_read_unlock();
local_irq_disable(); local_irq_disable();
input_queue_head_incr(sd); input_queue_head_incr(sd);
if (++work >= quota) { if (++work >= quota) {
...@@ -6135,6 +6138,7 @@ static void rollback_registered_many(struct list_head *head) ...@@ -6135,6 +6138,7 @@ static void rollback_registered_many(struct list_head *head)
unlist_netdevice(dev); unlist_netdevice(dev);
dev->reg_state = NETREG_UNREGISTERING; dev->reg_state = NETREG_UNREGISTERING;
on_each_cpu(flush_backlog, dev, 1);
} }
synchronize_net(); synchronize_net();
...@@ -6770,8 +6774,6 @@ void netdev_run_todo(void) ...@@ -6770,8 +6774,6 @@ void netdev_run_todo(void)
dev->reg_state = NETREG_UNREGISTERED; dev->reg_state = NETREG_UNREGISTERED;
on_each_cpu(flush_backlog, dev, 1);
netdev_wait_allrefs(dev); netdev_wait_allrefs(dev);
/* paranoia */ /* paranoia */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment