From 18041e31743d278b6323518d20a2ef656c3cc689 Mon Sep 17 00:00:00 2001 From: Nikolay Aleksandrov Date: Tue, 18 Aug 2015 21:40:16 +0300 Subject: [PATCH] vrf: vrf_master_ifindex_rcu is not always called with rcu read lock While running net-next I hit this: [ 634.073119] =============================== [ 634.073150] [ INFO: suspicious RCU usage. ] [ 634.073182] 4.2.0-rc6+ #45 Not tainted [ 634.073213] ------------------------------- [ 634.073244] include/net/vrf.h:38 suspicious rcu_dereference_check() usage! [ 634.073274] other info that might help us debug this: [ 634.073307] rcu_scheduler_active = 1, debug_locks = 1 [ 634.073338] 2 locks held by swapper/0/0: [ 634.073369] #0: (((&n->timer))){+.-...}, at: [] call_timer_fn+0x5/0x480 [ 634.073412] #1: (slock-AF_INET){+.-...}, at: [] icmp_send+0x155/0x5f0 [ 634.073450] stack backtrace: [ 634.073483] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc6+ #45 [ 634.073514] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 634.073545] 0000000000000000 0593ba8242d9ace4 ffff88002fc03b48 ffffffff81803f1b [ 634.073612] 0000000000000000 ffffffff81e12500 ffff88002fc03b78 ffffffff811003c5 [ 634.073642] 0000000000000000 ffff88002ec4e600 ffffffff81f00f80 ffff88002fc03cf0 [ 634.073669] Call Trace: [ 634.073694] [] dump_stack+0x4c/0x65 [ 634.073728] [] lockdep_rcu_suspicious+0xc5/0x100 [ 634.073763] [] icmp_route_lookup+0x176/0x5c0 [ 634.073793] [] ? icmp_send+0x35b/0x5f0 [ 634.073818] [] ? icmp_send+0x2d4/0x5f0 [ 634.073844] [] icmp_send+0x42e/0x5f0 [ 634.073873] [] ipv4_link_failure+0x22/0xa0 [ 634.073899] [] arp_error_report+0x3a/0x80 [ 634.073926] [] ? neigh_lookup+0x2c0/0x2c0 [ 634.073952] [] neigh_invalidate+0x8e/0x110 [ 634.073984] [] neigh_timer_handler+0x1ae/0x290 [ 634.074013] [] ? neigh_lookup+0x2c0/0x2c0 [ 634.074013] [] call_timer_fn+0xb3/0x480 [ 634.074013] [] ? call_timer_fn+0x5/0x480 [ 634.074013] [] ? neigh_lookup+0x2c0/0x2c0 [ 634.074013] [] run_timer_softirq+0x20c/0x430 [ 634.074013] [] __do_softirq+0xde/0x630 [ 634.074013] [] irq_exit+0x117/0x120 [ 634.074013] [] smp_apic_timer_interrupt+0x46/0x60 [ 634.074013] [] apic_timer_interrupt+0x70/0x80 [ 634.074013] [] ? native_safe_halt+0x6/0x10 [ 634.074013] [] ? trace_hardirqs_on+0xd/0x10 [ 634.074013] [] default_idle+0x23/0x200 [ 634.074013] [] arch_cpu_idle+0xf/0x20 [ 634.074013] [] default_idle_call+0x2a/0x40 [ 634.074013] [] cpu_startup_entry+0x39c/0x4c0 [ 634.074013] [] rest_init+0x13d/0x150 [ 634.074013] [] start_kernel+0x4a8/0x4c9 [ 634.074013] [] ? early_idt_handler_array+0x120/0x120 [ 634.074013] [] x86_64_start_reservations+0x2a/0x2c [ 634.074013] [] x86_64_start_kernel+0x14a/0x16d It would seem vrf_master_ifindex_rcu() can be called without RCU held in other contexts as well so introduce a new helper which acquires rcu and returns the ifindex. Also add curly braces around both the "if" and "else" parts as per the style guide. Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- include/net/vrf.h | 20 ++++++++++++++++++-- net/ipv4/icmp.c | 4 ++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/include/net/vrf.h b/include/net/vrf.h index 3bb4af462ed6..5bfb16237fd7 100644 --- a/include/net/vrf.h +++ b/include/net/vrf.h @@ -43,9 +43,9 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev) if (!dev) return 0; - if (netif_is_vrf(dev)) + if (netif_is_vrf(dev)) { ifindex = dev->ifindex; - else { + } else { vrf_ptr = rcu_dereference(dev->vrf_ptr); if (vrf_ptr) ifindex = vrf_ptr->ifindex; @@ -54,6 +54,17 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev) return ifindex; } +static inline int vrf_master_ifindex(const struct net_device *dev) +{ + int ifindex; + + rcu_read_lock(); + ifindex = vrf_master_ifindex_rcu(dev); + rcu_read_unlock(); + + return ifindex; +} + /* called with rcu_read_lock */ static inline int vrf_dev_table_rcu(const struct net_device *dev) { @@ -133,6 +144,11 @@ static inline int vrf_master_ifindex_rcu(const struct net_device *dev) return 0; } +static inline int vrf_master_ifindex(const struct net_device *dev) +{ + return 0; +} + static inline int vrf_dev_table_rcu(const struct net_device *dev) { return 0; diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index c6f1ce149ffb..f16488efa1c8 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -426,7 +426,7 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb) fl4.flowi4_mark = mark; fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos); fl4.flowi4_proto = IPPROTO_ICMP; - fl4.flowi4_oif = vrf_master_ifindex_rcu(skb->dev) ? : skb->dev->ifindex; + fl4.flowi4_oif = vrf_master_ifindex(skb->dev) ? : skb->dev->ifindex; security_skb_classify_flow(skb, flowi4_to_flowi(&fl4)); rt = ip_route_output_key(net, &fl4); if (IS_ERR(rt)) @@ -460,7 +460,7 @@ static struct rtable *icmp_route_lookup(struct net *net, fl4->flowi4_proto = IPPROTO_ICMP; fl4->fl4_icmp_type = type; fl4->fl4_icmp_code = code; - fl4->flowi4_oif = vrf_master_ifindex_rcu(skb_in->dev) ? : skb_in->dev->ifindex; + fl4->flowi4_oif = vrf_master_ifindex(skb_in->dev) ? : skb_in->dev->ifindex; security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4)); rt = __ip_route_output_key(net, fl4); -- GitLab