NULL pointer dereference in rt6_get_cookie()

Message ID	20151014061421.GD68583@kafai-mba.local
State	RFC, archived
Delegated to:	David Miller
Headers	show Return-Path: <netdev-owner@vger.kernel.org> Date: Tue, 13 Oct 2015 23:14:21 -0700 From: Martin KaFai Lau <kafai@fb.com> To: Phil Sutter <phil@nwl.cc> CC: <netdev@vger.kernel.org>, Hannes Frederic Sowa <hannes@stressinduktion.org>, Steffen Klassert <steffen.klassert@secunet.com>, Julian Anastasov <ja@ssi.bg> Subject: Re: NULL pointer dereference in rt6_get_cookie() Message-ID: <20151014061421.GD68583@kafai-mba.local> References: <20151010132437.GB25926@orbit.nwl.cc> <20151013181443.GB68583@kafai-mba.local> <20151013191039.GA3070@base.sg13b.nwl.cc> <20151013192543.06B5021398@mail.nwl.cc> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20151013192543.06B5021398@mail.nwl.cc> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk

Message ID

20151014061421.GD68583@kafai-mba.local

State

RFC, archived

Delegated to:

David Miller

Headers

Date: Tue, 13 Oct 2015 23:14:21 -0700
From: Martin KaFai Lau <kafai@fb.com>
To: Phil Sutter <phil@nwl.cc>
CC: <netdev@vger.kernel.org>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	Steffen Klassert <steffen.klassert@secunet.com>,
	Julian Anastasov <ja@ssi.bg>
Subject: Re: NULL pointer dereference in rt6_get_cookie()
Message-ID: <20151014061421.GD68583@kafai-mba.local>
References: <20151010132437.GB25926@orbit.nwl.cc>
	<20151013181443.GB68583@kafai-mba.local>
	<20151013191039.GA3070@base.sg13b.nwl.cc>
	<20151013192543.06B5021398@mail.nwl.cc>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20151013192543.06B5021398@mail.nwl.cc>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: netdev-owner@vger.kernel.org
Precedence: bulk

Commit Message

Martin KaFai Lau Oct. 14, 2015, 6:14 a.m. UTC

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

the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Phil Sutter Oct. 14, 2015, 10:34 p.m. UTC | #1

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin KaFai Lau Oct. 14, 2015, 11:17 p.m. UTC | #2

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--- 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);
 	}
--
To unsubscribe from this list: send the line "unsubscribe netdev" in

NULL pointer dereference in rt6_get_cookie()

Commit Message

Comments

Patch