linux.git/net/ipv6/ip6_fib.c, branch v4.2

ipv6: Fix a potential deadlock when creating pcpu rt

2015-08-17T21:28:03+00:00

rt6_make_pcpu_route() is called under read_lock(&table->tb6_lock).
rt6_make_pcpu_route() calls ip6_rt_pcpu_alloc(rt) which then
calls dst_alloc().  dst_alloc() _may_ call ip6_dst_gc() which takes
the write_lock(&tabl->tb6_lock).  A visualized version:

read_lock(&table->tb6_lock);
rt6_make_pcpu_route();
=> ip6_rt_pcpu_alloc();
=> dst_alloc();
=> ip6_dst_gc();
=> write_lock(&table->tb6_lock); /* oops */

The fix is to do a read_unlock first before calling ip6_rt_pcpu_alloc().

A reported stack:

[141625.537638] INFO: rcu_sched self-detected stall on CPU { 27}  (t=60000 jiffies g=4159086 c=4159085 q=2139)
[141625.547469] Task dump for CPU 27:
[141625.550881] mtr             R  running task        0 22121  22081 0x00000008
[141625.558069]  0000000000000000 ffff88103f363d98 ffffffff8106e488 000000000000001b
[141625.565641]  ffffffff81684900 ffff88103f363db8 ffffffff810702b0 0000000008000000
[141625.573220]  ffffffff81684900 ffff88103f363de8 ffffffff8108df9f ffff88103f375a00
[141625.580803] Call Trace:
[141625.583345]    [] sched_show_task+0xc1/0xc6
[141625.589650]  [] dump_cpu_task+0x35/0x39
[141625.595144]  [] rcu_dump_cpu_stacks+0x6a/0x8c
[141625.601320]  [] rcu_check_callbacks+0x1f6/0x5d4
[141625.607669]  [] update_process_times+0x2a/0x4f
[141625.613925]  [] tick_sched_handle+0x32/0x3e
[141625.619923]  [] tick_sched_timer+0x35/0x5c
[141625.625830]  [] __hrtimer_run_queues+0x8f/0x18d
[141625.632171]  [] hrtimer_interrupt+0xa0/0x166
[141625.638258]  [] local_apic_timer_interrupt+0x4e/0x52
[141625.645036]  [] smp_apic_timer_interrupt+0x39/0x4a
[141625.651643]  [] apic_timer_interrupt+0x68/0x70
[141625.657895]    [] ? dst_destroy+0x7c/0xb5
[141625.664188]  [] ? fib6_flush_trees+0x20/0x20
[141625.670272]  [] ? queue_write_lock_slowpath+0x60/0x6f
[141625.677140]  [] _raw_write_lock_bh+0x23/0x25
[141625.683218]  [] __fib6_clean_all+0x40/0x82
[141625.689124]  [] ? fib6_flush_trees+0x20/0x20
[141625.695207]  [] fib6_clean_all+0xe/0x10
[141625.700854]  [] fib6_run_gc+0x79/0xc8
[141625.706329]  [] ip6_dst_gc+0x85/0xf9
[141625.711718]  [] dst_alloc+0x55/0x159
[141625.717105]  [] __ip6_dst_alloc.isra.32+0x19/0x63
[141625.723620]  [] ip6_pol_route+0x36a/0x3e8
[141625.729441]  [] ip6_pol_route_output+0x11/0x13
[141625.735700]  [] fib6_rule_action+0xa7/0x1bf
[141625.741698]  [] ? ip6_pol_route_input+0x17/0x17
[141625.748043]  [] fib_rules_lookup+0xb5/0x12a
[141625.754050]  [] ? poll_select_copy_remaining+0xf9/0xf9
[141625.761002]  [] fib6_rule_lookup+0x37/0x5c
[141625.766914]  [] ? ip6_pol_route_input+0x17/0x17
[141625.773260]  [] ip6_route_output+0x7a/0x82
[141625.779177]  [] ip6_dst_lookup_tail+0x53/0x112
[141625.785437]  [] ip6_dst_lookup_flow+0x2a/0x6b
[141625.791604]  [] rawv6_sendmsg+0x407/0x9b6
[141625.797423]  [] ? do_ipv6_setsockopt.isra.8+0xd87/0xde2
[141625.804464]  [] inet_sendmsg+0x57/0x8e
[141625.810028]  [] sock_sendmsg+0x2e/0x3c
[141625.815588]  [] SyS_sendto+0xfe/0x143
[141625.821063]  [] ? rawv6_setsockopt+0x5e/0x67
[141625.827146]  [] ? sock_common_setsockopt+0xf/0x11
[141625.833660]  [] ? SyS_setsockopt+0x81/0xa2
[141625.839565]  [] entry_SYSCALL_64_fastpath+0x12/0x6a

Fixes: d52d3997f843 ("pv6: Create percpu rt6_info")
Signed-off-by: Martin KaFai Lau 
CC: Hannes Frederic Sowa 
Reported-by: Steinar H. Gunderson 
Signed-off-by: David S. Miller

ipv6: Create percpu rt6_info

2015-05-25T17:25:35+00:00

After the patch
'ipv6: Only create RTF_CACHE routes after encountering pmtu exception',
we need to compensate the performance hit (bouncing dst->__refcnt).

Signed-off-by: Martin KaFai Lau 
Cc: Hannes Frederic Sowa 
Cc: Steffen Klassert 
Cc: Julian Anastasov 
Signed-off-by: David S. Miller

ipv6: Only create RTF_CACHE routes after encountering pmtu exception

2015-05-25T17:25:33+00:00

This patch creates a RTF_CACHE routes only after encountering a pmtu
exception.

After ip6_rt_update_pmtu() has inserted the RTF_CACHE route to the fib6
tree, the rt->rt6i_node->fn_sernum is bumped which will fail the
ip6_dst_check() and trigger a relookup.

Signed-off-by: Martin KaFai Lau 
Cc: Hannes Frederic Sowa 
Cc: Steffen Klassert 
Cc: Julian Anastasov 
Signed-off-by: David S. Miller

ipv6: fix ECMP route replacement

2015-05-20T16:02:26+00:00

When replacing an IPv6 multipath route with "ip route replace", i.e.
NLM_F_CREATE | NLM_F_REPLACE, fib6_add_rt2node() replaces only first
matching route without fixing its siblings, resulting in corrupted
siblings linked list; removing one of the siblings can then end in an
infinite loop.

IPv6 ECMP implementation is a bit different from IPv4 so that route
replacement cannot work in exactly the same way. This should be a
reasonable approximation:

1. If the new route is ECMP-able and there is a matching ECMP-able one
already, replace it and all its siblings (if any).

2. If the new route is ECMP-able and no matching ECMP-able route exists,
replace first matching non-ECMP-able (if any) or just add the new one.

3. If the new route is not ECMP-able, replace first matching
non-ECMP-able route (if any) or add the new route.

We also need to remove the NLM_F_REPLACE flag after replacing old
route(s) by first nexthop of an ECMP route so that each subsequent
nexthop does not replace previous one.

Fixes: 51ebd3181572 ("ipv6: add support of equal cost multipath (ECMP)")
Signed-off-by: Michal Kubecek 
Acked-by: Nicolas Dichtel 
Signed-off-by: David S. Miller

ipv6: coding style: comparison for inequality with NULL

2015-03-31T17:51:54+00:00

The ipv6 code uses a mixture of coding styles. In some instances check for NULL
pointer is done as x != NULL and sometimes as x. x is preferred according to
checkpatch and this patch makes the code consistent by adopting the latter
form.

No changes detected by objdiff.

Signed-off-by: Ian Morris 
Signed-off-by: David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net

2015-01-28T00:59:56+00:00

Conflicts:
	arch/arm/boot/dts/imx6sx-sdb.dts
	net/sched/cls_bpf.c

Two simple sets of overlapping changes.

Signed-off-by: David S. Miller

ipv6: replacing a rt6_info needs to purge possible propagated rt6_infos too

2015-01-27T08:22:14+00:00

Lubomir Rintel reported that during replacing a route the interface
reference counter isn't correctly decremented.

To quote bug :
| [root@rhel7-5 lkundrak]# sh -x lal
| + ip link add dev0 type dummy
| + ip link set dev0 up
| + ip link add dev1 type dummy
| + ip link set dev1 up
| + ip addr add 2001:db8:8086::2/64 dev dev0
| + ip route add 2001:db8:8086::/48 dev dev0 proto static metric 20
| + ip route add 2001:db8:8088::/48 dev dev1 proto static metric 10
| + ip route replace 2001:db8:8086::/48 dev dev1 proto static metric 20
| + ip link del dev0 type dummy
| Message from syslogd@rhel7-5 at Jan 23 10:54:41 ...
|  kernel:unregister_netdevice: waiting for dev0 to become free. Usage count = 2
|
| Message from syslogd@rhel7-5 at Jan 23 10:54:51 ...
|  kernel:unregister_netdevice: waiting for dev0 to become free. Usage count = 2

During replacement of a rt6_info we must walk all parent nodes and check
if the to be replaced rt6_info got propagated. If so, replace it with
an alive one.

Fixes: 4a287eba2de3957 ("IPv6 routing, NLM_F_* flag support: REPLACE and EXCL flags support, warn about missing CREATE flag")
Reported-by: Lubomir Rintel 
Signed-off-by: Hannes Frederic Sowa 
Tested-by: Lubomir Rintel 
Signed-off-by: David S. Miller

netlink: make nlmsg_end() and genlmsg_end() void

2015-01-18T06:03:45+00:00

Contrary to common expectations for an "int" return, these functions
return only a positive value -- if used correctly they cannot even
return 0 because the message header will necessarily be in the skb.

This makes the very common pattern of

  if (genlmsg_end(...) < 0) { ... }

be a whole bunch of dead code. Many places also simply do

  return nlmsg_end(...);

and the caller is expected to deal with it.

This also commonly (at least for me) causes errors, because it is very
common to write

  if (my_function(...))
    /* error condition */

and if my_function() does "return nlmsg_end()" this is of course wrong.

Additionally, there's not a single place in the kernel that actually
needs the message length returned, and if anyone needs it later then
it'll be very easy to just use skb->len there.

Remove this, and make the functions void. This removes a bunch of dead
code as described above. The patch adds lines because I did

-	return nlmsg_end(...);
+	nlmsg_end(...);
+	return 0;

I could have preserved all the function's return values by returning
skb->len, but instead I've audited all the places calling the affected
functions and found that none cared. A few places actually compared
the return value with <= 0 in dump functionality, but that could just
be changed to < 0 with no change in behaviour, so I opted for the more
efficient version.

One instance of the error I've made numerous times now is also present
in net/phonet/pn_netlink.c in the route_dumpit() function - it didn't
check for <0 or <=0 and thus broke out of the loop every single time.
I've preserved this since it will (I think) have caused the messages to
userspace to be formatted differently with just a single message for
every SKB returned to userspace. It's possible that this isn't needed
for the tools that actually use this, but I don't even know what they
are so couldn't test that changing this behaviour would be acceptable.

Signed-off-by: Johannes Berg 
Signed-off-by: David S. Miller

net: fib6: convert cfg metric to u32 outside of table write lock

2015-01-06T03:55:24+00:00

Do the nla validation earlier, outside the write lock.

This is needed by followup patch which needs to be able to call
request_module (which can sleep) if needed.

Joint work with Daniel Borkmann.

Signed-off-by: Daniel Borkmann 
Signed-off-by: Florian Westphal 
Signed-off-by: David S. Miller

net: fib6: fib6_commit_metrics: fix potential NULL pointer dereference

2015-01-06T03:55:24+00:00

When IPv6 host routes with metrics attached are being added, we fetch
the metrics store from the dst via COW through dst_metrics_write_ptr(),
added through commit e5fd387ad5b3.

One remaining problem here is that we actually call into inet_getpeer()
and may end up allocating/creating a new peer from the kmemcache, which
may fail.

Example trace from perf probe (inet_getpeer:41) where create is 1:

ip 6877 [002] 4221.391591: probe:inet_getpeer: (ffffffff8165e293)
  85e294 inet_getpeer.part.7 (<- kmem_cache_alloc())
  85e578 inet_getpeer
  8eb333 ipv6_cow_metrics
  8f10ff fib6_commit_metrics

Therefore, a check for NULL on the return of dst_metrics_write_ptr()
is necessary here.

Joint work with Florian Westphal.

Fixes: e5fd387ad5b3 ("ipv6: do not overwrite inetpeer metrics prematurely")
Cc: Michal Kubeček 
Signed-off-by: Florian Westphal 
Signed-off-by: Daniel Borkmann 
Signed-off-by: David S. Miller