All of lore.kernel.org
 help / color / mirror / Atom feed
* NULL pointer dereference in rt6_get_cookie()
@ 2015-10-10 13:24 Phil Sutter
  2015-10-12 20:31 ` Martin KaFai Lau
  2015-10-13 18:14 ` Martin KaFai Lau
  0 siblings, 2 replies; 9+ messages in thread
From: Phil Sutter @ 2015-10-10 13:24 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

Hi,

Linux 4.2.0 and above dereferences a NULL pointer for me when sending an
IPsec secured packet for the first time. I use kernel IPsec with racoon
and setkey. This is what my configuration looks like:

Local host: 2001:4dd0:ff3b:13::23
Remote host: 2001:4dd0:ff3b:13::5

Setkey instructions:

| # allow for ND
| spdadd ::/0 ::/0 icmp6 -P in prio def + 100 none;
| spdadd ::/0 ::/0 icmp6 -P out prio def + 100 none;
| 
| # transport mode
| spdadd 2001:4dd0:ff3b:13::23 2001:4dd0:ff3b:13::5 any -P out
|         ipsec esp/transport//require;
| spdadd 2001:4dd0:ff3b:13::5 2001:4dd0:ff3b:13::23 any -P in
|         ipsec esp/transport//require;

After booting the local system, a simple call to ping6 suffices to
trigger the bug:

| # ping6 2001:4dd0:ff3b:13::5

This is the kernel log:

| BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
| IP: [<ffffffff8171a95e>] __ip6_datagram_connect+0x71e/0xa20
| PGD c2cb1067 PUD c2d7a067 PMD 0 
| Oops: 0000 [#1] PREEMPT SMP 
| Modules linked in: cmac nfs lockd grace sunrpc bridge stp llc nvidia(PO) snd_usb_audio snd_usbmidi_lib iTCO_wdt
| CPU: 1 PID: 2964 Comm: ping6 Tainted: P           O    4.2.1-aufs #10
| Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./4Core1333-Viiv, BIOS P1.60 07/01/2008
| task: ffff8800ca62bc00 ti: ffff880129a14000 task.ti: ffff880129a14000
| RIP: 0010:[<ffffffff8171a95e>]  [<ffffffff8171a95e>] __ip6_datagram_connect+0x71e/0xa20
| RSP: 0018:ffff880129a17da8  EFLAGS: 00010296
| RAX: 000000000000000b RBX: 0000000000000000 RCX: 0000000000000006
| RDX: 0000000000000007 RSI: 0000000000000246 RDI: ffff88012fc8d5a0
| RBP: ffff8800cb9a9048 R08: 756e207369207472 R09: 216c6c756e207369
| R10: 0000000000000665 R11: 0000000000000006 R12: ffff8800cb9a8cf8
| R13: ffff8800cb9a8cf8 R14: 0000000000000000 R15: ffff8800cb9a8cc0
| FS:  00007fb76ad74700(0000) GS:ffff88012fc80000(0000) knlGS:0000000000000000
| CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
| CR2: 00000000000000a0 CR3: 00000000c2dba000 CR4: 00000000000406e0
| Stack:
|  ffff8800cb9a9048 ffff8800cb9a8de0 ffff8800cb9feb70 ffffffff816b2c41
|  00007fb70000000b ffffea0000df7200 ffff8800cb9f5cfc ffff8800cb9a8cc0
|  03fffffffe068a20 ffff8800cb9a8cc0 ffffffff817097c0 0000000100000000
| Call Trace:
|  [<ffffffff816b2c41>] ? udp_lib_get_port+0x1a1/0x380
|  [<ffffffff817097c0>] ? udpv6_rcv+0x20/0x20
|  [<ffffffff8171ac82>] ? ip6_datagram_connect+0x22/0x40
|  [<ffffffff8163ae9b>] ? SyS_connect+0x6b/0xb0
|  [<ffffffff810767ac>] ? __do_page_fault+0x15c/0x380
|  [<ffffffff8163a8d3>] ? SyS_socket+0x63/0xa0
|  [<ffffffff81741957>] ? entry_SYSCALL_64_fastpath+0x12/0x6a
| Code: ba ae 00 00 00 48 c7 c6 7b 71 94 81 48 c7 c7 63 71 94 81 e8 6c 0f 02 00 48 85 db 75 0e 48 c7 c7 9f 71 94 81 31 c0 e8 59 0f 02 00 <48> 83 bb a0 00 00 00 00 75 0e 48 c7 c7 ae 71 94 81 31 c0 e8 41 
| RIP  [<ffffffff8171a95e>] __ip6_datagram_connect+0x71e/0xa20
|  RSP <ffff880129a17da8>
| CR2: 00000000000000a0
| ---[ end trace a8591fc98c30e10f ]---
| note: ping6[2964] exited with preempt_count 1
| BUG: scheduling while atomic: ping6/2964/0x00000002
| Modules linked in: cmac nfs lockd grace sunrpc bridge stp llc nvidia(PO) snd_usb_audio snd_usbmidi_lib iTCO_wdt
| CPU: 1 PID: 2964 Comm: ping6 Tainted: P      D    O    4.2.1-aufs #10
| Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./4Core1333-Viiv, BIOS P1.60 07/01/2008
|  0000000000000000 ffff8800ca62bc00 ffffffff8173c728 ffff88012fc940c0
|  ffffffff810f8130 0000000000000002 ffffffff8173e578 ffff8800c2d323c8
|  0000000000000202 0000000000000001 ffff880129a17a88 ffff880129a17aa8
| Call Trace:
|  [<ffffffff8173c728>] ? dump_stack+0x47/0x67
|  [<ffffffff810f8130>] ? __schedule_bug+0x40/0x50
|  [<ffffffff8173e578>] ? __schedule+0x6d8/0x830
|  [<ffffffff8173e701>] ? schedule+0x31/0x80
|  [<ffffffff8163cec4>] ? __lock_sock+0x64/0x90
|  [<ffffffff8110b9d0>] ? wait_woken+0x80/0x80
|  [<ffffffff8163cf3a>] ? lock_sock_nested+0x4a/0x50
|  [<ffffffff8170bb1b>] ? udpv6_destroy_sock+0xb/0x50
|  [<ffffffff8163eb28>] ? sk_common_release+0x18/0xf0
|  [<ffffffff816be618>] ? inet_release+0x38/0x60
|  [<ffffffff81639499>] ? sock_release+0x19/0x80
|  [<ffffffff8163950d>] ? sock_close+0xd/0x20
|  [<ffffffff811bdadf>] ? __fput+0x8f/0x1d0
|  [<ffffffff810f2bb5>] ? task_work_run+0x95/0xb0
|  [<ffffffff810ddff7>] ? do_exit+0x357/0x9f0
|  [<ffffffff81041fb6>] ? oops_end+0x66/0x90
|  [<ffffffff81076058>] ? no_context+0x128/0x350
|  [<ffffffff81118594>] ? wake_up_klogd+0x34/0x50
|  [<ffffffff81742f82>] ? page_fault+0x22/0x30
|  [<ffffffff8171a95e>] ? __ip6_datagram_connect+0x71e/0xa20
|  [<ffffffff8171a95e>] ? __ip6_datagram_connect+0x71e/0xa20
|  [<ffffffff816b2c41>] ? udp_lib_get_port+0x1a1/0x380
|  [<ffffffff817097c0>] ? udpv6_rcv+0x20/0x20
|  [<ffffffff8171ac82>] ? ip6_datagram_connect+0x22/0x40
|  [<ffffffff8163ae9b>] ? SyS_connect+0x6b/0xb0
|  [<ffffffff810767ac>] ? __do_page_fault+0x15c/0x380
|  [<ffffffff8163a8d3>] ? SyS_socket+0x63/0xa0
|  [<ffffffff81741957>] ? entry_SYSCALL_64_fastpath+0x12/0x6a

I assume the second BUG is just a follow-up and therefore not
interesting. Using printk-debugging I could track down the problem to
rt6_get_cookie() function in include/net/ip6_fib.h:

The conditional at the start of the function evaluates true, since
'rt->rt6i_flags & RTF_PCPU' is non-zero. Due to that, 'rt' pointer is
reassigned:

| rt = (struct rt6_info *)(rt->dst.from);

It appears that this effectively assigns NULL to it, and the following
dereference causes the bug. This seems to be the culprit change:

| commit d52d3997f843ffefaa8d8462790ffcaca6c74192
| Author: Martin KaFai Lau <kafai@fb.com>
| Date:   Fri May 22 20:56:06 2015 -0700
| 
|     ipv6: Create percpu rt6_info
|     
|     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 <kafai@fb.com>
|     Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
|     Cc: Steffen Klassert <steffen.klassert@secunet.com>
|     Cc: Julian Anastasov <ja@ssi.bg>
|     Signed-off-by: David S. Miller <davem@davemloft.net>

Kernel version 4.1.9 and below is not affected (the above commit exists
only in versions 4.2.0 and above).

Feel free to send me patches for testing, reproducing the problem is no
big deal for me.

Cheers, Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-10 13:24 NULL pointer dereference in rt6_get_cookie() Phil Sutter
@ 2015-10-12 20:31 ` Martin KaFai Lau
  2015-10-13 18:14 ` Martin KaFai Lau
  1 sibling, 0 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2015-10-12 20:31 UTC (permalink / raw)
  To: Phil Sutter
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

On Sat, Oct 10, 2015 at 03:24:37PM +0200, Phil Sutter wrote:
> Using printk-debugging I could track down the problem to
> rt6_get_cookie() function in include/net/ip6_fib.h:
>
> The conditional at the start of the function evaluates true, since
> 'rt->rt6i_flags & RTF_PCPU' is non-zero. Due to that, 'rt' pointer is
> reassigned:
>
> | rt = (struct rt6_info *)(rt->dst.from);
>
> It appears that this effectively assigns NULL to it, and the following
> dereference causes the bug.
Thanks for the report.  I am looking into it.

--Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-10 13:24 NULL pointer dereference in rt6_get_cookie() Phil Sutter
  2015-10-12 20:31 ` Martin KaFai Lau
@ 2015-10-13 18:14 ` Martin KaFai Lau
  2015-10-13 19:10   ` Phil Sutter
       [not found]   ` <20151013191039.GA3070@base.sg13b.nwl.cc>
  1 sibling, 2 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2015-10-13 18:14 UTC (permalink / raw)
  To: Phil Sutter
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

On Sat, Oct 10, 2015 at 03:24:37PM +0200, Phil Sutter wrote:
> The conditional at the start of the function evaluates true, since
> 'rt->rt6i_flags & RTF_PCPU' is non-zero.
Hi Phil, can you try the following patch and capture the dmesg output
to confirm the value of rt->rt6i_flags and the rt->dst.flags.

Thanks,
Martin

--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -167,8 +167,15 @@ static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)

 static inline u32 rt6_get_cookie(const struct rt6_info *rt)
 {
-	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE))
+	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE)) {
 		rt = (struct rt6_info *)(rt->dst.from);
+		if (!rt)
+			pr_err("rt6i_dst:%pI6c/%d rt6i_gateway:%pI6c "
+			       "rt6i_flags:%08X dst.flags:%08X\n",
+			       &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
+			       &rt->rt6i_gateway, rt->rt6i_flags,
+			       rt->dst.flags);
+	}

 	return rt->rt6i_node ? rt->rt6i_node->fn_sernum : 0;
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-13 18:14 ` Martin KaFai Lau
@ 2015-10-13 19:10   ` Phil Sutter
  2015-10-13 19:30     ` Martin KaFai Lau
       [not found]   ` <20151013191039.GA3070@base.sg13b.nwl.cc>
  1 sibling, 1 reply; 9+ messages in thread
From: Phil Sutter @ 2015-10-13 19:10 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

Hi Martin,

On Tue, Oct 13, 2015 at 11:14:43AM -0700, Martin KaFai Lau wrote:
> On Sat, Oct 10, 2015 at 03:24:37PM +0200, Phil Sutter wrote:
> > The conditional at the start of the function evaluates true, since
> > 'rt->rt6i_flags & RTF_PCPU' is non-zero.
> Hi Phil, can you try the following patch and capture the dmesg output
> to confirm the value of rt->rt6i_flags and the rt->dst.flags.
> 
> Thanks,
> Martin
> 
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -167,8 +167,15 @@ static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
> 
>  static inline u32 rt6_get_cookie(const struct rt6_info *rt)
>  {
> -	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE))
> +	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE)) {
>  		rt = (struct rt6_info *)(rt->dst.from);
> +		if (!rt)
> +			pr_err("rt6i_dst:%pI6c/%d rt6i_gateway:%pI6c "
> +			       "rt6i_flags:%08X dst.flags:%08X\n",
> +			       &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
> +			       &rt->rt6i_gateway, rt->rt6i_flags,
> +			       rt->dst.flags);
> +	}
> 
>  	return rt->rt6i_node ? rt->rt6i_node->fn_sernum : 0;
>  }

This code is not sane. Your pr_err() statement tries to dereference the
NULL pointer in question. Are you interested in the originally passed
rt6_info?

Cheers, Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
       [not found]   ` <20151013191039.GA3070@base.sg13b.nwl.cc>
@ 2015-10-13 19:26     ` Phil Sutter
  2015-10-14  6:14       ` Martin KaFai Lau
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Sutter @ 2015-10-13 19:26 UTC (permalink / raw)
  To: Martin KaFai Lau, netdev, Hannes Frederic Sowa, Steffen Klassert,
	Julian Anastasov

On Tue, Oct 13, 2015 at 09:10:39PM +0200, Phil Sutter wrote:
> Hi Martin,
> 
> On Tue, Oct 13, 2015 at 11:14:43AM -0700, Martin KaFai Lau wrote:
> > On Sat, Oct 10, 2015 at 03:24:37PM +0200, Phil Sutter wrote:
> > > The conditional at the start of the function evaluates true, since
> > > 'rt->rt6i_flags & RTF_PCPU' is non-zero.
> > Hi Phil, can you try the following patch and capture the dmesg output
> > to confirm the value of rt->rt6i_flags and the rt->dst.flags.
> > 
> > Thanks,
> > Martin
> > 
> > --- a/include/net/ip6_fib.h
> > +++ b/include/net/ip6_fib.h
> > @@ -167,8 +167,15 @@ static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
> > 
> >  static inline u32 rt6_get_cookie(const struct rt6_info *rt)
> >  {
> > -	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE))
> > +	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE)) {
> >  		rt = (struct rt6_info *)(rt->dst.from);
> > +		if (!rt)
> > +			pr_err("rt6i_dst:%pI6c/%d rt6i_gateway:%pI6c "
> > +			       "rt6i_flags:%08X dst.flags:%08X\n",
> > +			       &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
> > +			       &rt->rt6i_gateway, rt->rt6i_flags,
> > +			       rt->dst.flags);
> > +	}
> > 
> >  	return rt->rt6i_node ? rt->rt6i_node->fn_sernum : 0;
> >  }
> 
> This code is not sane. Your pr_err() statement tries to dereference the
> NULL pointer in question. Are you interested in the originally passed
> rt6_info?

I have backed up the rt pointer at top of the function and restored it
before pr_err, this is the output:

| rt6i_dst:2001:4dd0:ff3b:13::/64 rt6i_gateway::: rt6i_flags:40000001 dst.flags:00000000

HTH, Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-13 19:10   ` Phil Sutter
@ 2015-10-13 19:30     ` Martin KaFai Lau
  0 siblings, 0 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2015-10-13 19:30 UTC (permalink / raw)
  To: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

On Tue, Oct 13, 2015 at 09:10:39PM +0200, Phil Sutter wrote:
> Hi Martin,
>
> On Tue, Oct 13, 2015 at 11:14:43AM -0700, Martin KaFai Lau wrote:
> > On Sat, Oct 10, 2015 at 03:24:37PM +0200, Phil Sutter wrote:
> > > The conditional at the start of the function evaluates true, since
> > > 'rt->rt6i_flags & RTF_PCPU' is non-zero.
> > Hi Phil, can you try the following patch and capture the dmesg output
> > to confirm the value of rt->rt6i_flags and the rt->dst.flags.
> >
> > Thanks,
> > Martin
> >
> > --- a/include/net/ip6_fib.h
> > +++ b/include/net/ip6_fib.h
> > @@ -167,8 +167,15 @@ static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
> >
> >  static inline u32 rt6_get_cookie(const struct rt6_info *rt)
> >  {
> > -	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE))
> > +	if (rt->rt6i_flags & RTF_PCPU || unlikely(rt->dst.flags & DST_NOCACHE)) {
> >  		rt = (struct rt6_info *)(rt->dst.from);
> > +		if (!rt)
> > +			pr_err("rt6i_dst:%pI6c/%d rt6i_gateway:%pI6c "
> > +			       "rt6i_flags:%08X dst.flags:%08X\n",
> > +			       &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
> > +			       &rt->rt6i_gateway, rt->rt6i_flags,
> > +			       rt->dst.flags);
> > +	}
> >
> >  	return rt->rt6i_node ? rt->rt6i_node->fn_sernum : 0;
> >  }
>
> This code is not sane. Your pr_err() statement tries to dereference the
> NULL pointer in question. Are you interested in the originally passed
> rt6_info?
Good catch. sorry about that.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-13 19:26     ` Phil Sutter
@ 2015-10-14  6:14       ` Martin KaFai Lau
  2015-10-14 22:34         ` Phil Sutter
  0 siblings, 1 reply; 9+ messages in thread
From: Martin KaFai Lau @ 2015-10-14  6:14 UTC (permalink / raw)
  To: Phil Sutter
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

On Tue, Oct 13, 2015 at 09:26:41PM +0200, Phil Sutter wrote:
> I have backed up the rt pointer at top of the function and restored it
> before pr_err, this is the output:
>
> | rt6i_dst:2001:4dd0:ff3b:13::/64 rt6i_gateway::: rt6i_flags:40000001 dst.flags:00000000
Hi Phil, Can you try the following patch and report the pr_err?

Thanks,
Martin

--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -262,7 +262,7 @@ static struct dst_ops ip6_dst_blackhole_ops = {
 	.default_advmss		=	ip6_default_advmss,
 	.update_pmtu		=	ip6_rt_blackhole_update_pmtu,
 	.redirect		=	ip6_rt_blackhole_redirect,
-	.cow_metrics		=	ip6_rt_blackhole_cow_metrics,
+	.cow_metrics		=	dst_cow_metrics_generic,
 	.neigh_lookup		=	ip6_neigh_lookup,
 };

@@ -1201,21 +1201,20 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		new = &rt->dst;

 		memset(new + 1, 0, sizeof(*rt) - sizeof(*new));
+		INIT_LIST_HEAD(&rt->rt6i_siblings);
+		INIT_LIST_HEAD(&rt->rt6i_uncached);

 		new->__use = 1;
 		new->input = dst_discard;
 		new->output = dst_discard_out;

-		if (dst_metrics_read_only(&ort->dst))
-			new->_metrics = ort->dst._metrics;
-		else
-			dst_copy_metrics(new, &ort->dst);
+		dst_copy_metrics(new, &ort->dst);
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);

 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags;
+		rt->rt6i_flags = ort->rt6i_flags & (~RTF_PCPU);
 		rt->rt6i_metric = 0;

 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1223,6 +1222,19 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		memcpy(&rt->rt6i_src, &ort->rt6i_src, sizeof(struct rt6key));
 #endif

+		pr_err("ort:%p rt6i_dst:[%pI6c]/%d rt6i_gateway:[%pI6c] "
+		       "rt6i_flags:%08X dst.flags:%08X\n",
+		       ort,
+		       &ort->rt6i_dst.addr, ort->rt6i_dst.plen,
+		       &ort->rt6i_gateway, ort->rt6i_flags,
+		       ort->dst.flags);
+		pr_err(" rt:%p rt6i_dst:[%pI6c]/%d rt6i_gateway:[%pI6c] "
+		       "rt6i_flags:%08X dst.flags:%08X\n",
+		       rt,
+		       &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
+		       &rt->rt6i_gateway, rt->rt6i_flags,
+		       rt->dst.flags);
+
 		dst_free(new);
 	}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-14  6:14       ` Martin KaFai Lau
@ 2015-10-14 22:34         ` Phil Sutter
  2015-10-14 23:17           ` Martin KaFai Lau
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Sutter @ 2015-10-14 22:34 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

Hi Martin,

On Tue, Oct 13, 2015 at 11:14:21PM -0700, Martin KaFai Lau wrote:
> On Tue, Oct 13, 2015 at 09:26:41PM +0200, Phil Sutter wrote:
> > I have backed up the rt pointer at top of the function and restored it
> > before pr_err, this is the output:
> >
> > | rt6i_dst:2001:4dd0:ff3b:13::/64 rt6i_gateway::: rt6i_flags:40000001 dst.flags:00000000
> Hi Phil, Can you try the following patch and report the pr_err?

Probably needless to say, but with your patch applied the Oops does not
occur anymore. This is the log output:

| [   46.518869] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   46.518874] IPv6:  rt:ffff8800cb07a000 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   46.529171] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   46.529174] IPv6:  rt:ffff8800cb07b500 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   46.529187] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   46.529189] IPv6:  rt:ffff8800cb07ad80 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   47.532014] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   47.532021] IPv6:  rt:ffff8800cb07a000 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   47.532028] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   47.532031] IPv6:  rt:ffff8800cb07b500 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   49.536010] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   49.536014] IPv6:  rt:ffff8800cb07ad80 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   49.536021] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   49.536024] IPv6:  rt:ffff8800cb07a180 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   53.544013] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   53.544020] IPv6:  rt:ffff8800cb07a300 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
| [   53.544028] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
| [   53.544031] IPv6:  rt:ffff8800cb07b980 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000

In case the amount of log entries is surprising: my test-case is
mounting two NFS shares over IPsec. No idea if that's relevant or not.

Cheers, Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: NULL pointer dereference in rt6_get_cookie()
  2015-10-14 22:34         ` Phil Sutter
@ 2015-10-14 23:17           ` Martin KaFai Lau
  0 siblings, 0 replies; 9+ messages in thread
From: Martin KaFai Lau @ 2015-10-14 23:17 UTC (permalink / raw)
  To: Phil Sutter
  Cc: netdev, Hannes Frederic Sowa, Steffen Klassert, Julian Anastasov

On Thu, Oct 15, 2015 at 12:34:13AM +0200, Phil Sutter wrote:
> Hi Martin,
>
> On Tue, Oct 13, 2015 at 11:14:21PM -0700, Martin KaFai Lau wrote:
> > On Tue, Oct 13, 2015 at 09:26:41PM +0200, Phil Sutter wrote:
> > > I have backed up the rt pointer at top of the function and restored it
> > > before pr_err, this is the output:
> > >
> > > | rt6i_dst:2001:4dd0:ff3b:13::/64 rt6i_gateway::: rt6i_flags:40000001 dst.flags:00000000
> > Hi Phil, Can you try the following patch and report the pr_err?
>
> Probably needless to say, but with your patch applied the Oops does not
> occur anymore. This is the log output:
Thanks for testing it.  The patch may need a bit refactoring work and
I will post it soon.

>
> | [   46.518869] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   46.518874] IPv6:  rt:ffff8800cb07a000 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   46.529171] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   46.529174] IPv6:  rt:ffff8800cb07b500 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   46.529187] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   46.529189] IPv6:  rt:ffff8800cb07ad80 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   47.532014] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   47.532021] IPv6:  rt:ffff8800cb07a000 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   47.532028] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   47.532031] IPv6:  rt:ffff8800cb07b500 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   49.536010] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   49.536014] IPv6:  rt:ffff8800cb07ad80 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   49.536021] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   49.536024] IPv6:  rt:ffff8800cb07a180 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   53.544013] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   53.544020] IPv6:  rt:ffff8800cb07a300 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
> | [   53.544028] IPv6: ort:ffff8800cbb5b800 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:40000001 dst.flags:00000020
> | [   53.544031] IPv6:  rt:ffff8800cb07b980 rt6i_dst:[2001:4dd0:ff3b:13::]/64 rt6i_gateway:[::] rt6i_flags:00000001 dst.flags:00000000
>
> In case the amount of log entries is surprising: my test-case is
> mounting two NFS shares over IPsec. No idea if that's relevant or not.
I also don't know why xfrm_lookup() errors out and then triggers
make_blackhole() but I believe it should not affect the fix here.

Thanks,
Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-10-14 23:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-10 13:24 NULL pointer dereference in rt6_get_cookie() Phil Sutter
2015-10-12 20:31 ` Martin KaFai Lau
2015-10-13 18:14 ` Martin KaFai Lau
2015-10-13 19:10   ` Phil Sutter
2015-10-13 19:30     ` Martin KaFai Lau
     [not found]   ` <20151013191039.GA3070@base.sg13b.nwl.cc>
2015-10-13 19:26     ` Phil Sutter
2015-10-14  6:14       ` Martin KaFai Lau
2015-10-14 22:34         ` Phil Sutter
2015-10-14 23:17           ` Martin KaFai Lau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.