Patchwork ipv6: release referenct of ip6_null_entry's dst entry in __ip6_del_rt

login
register
mail settings
Submitter Gao feng
Date Sept. 20, 2012, 5:25 a.m.
Message ID <1348118734-2967-1-git-send-email-gaofeng@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/185310/
State Accepted
Delegated to: David Miller
Headers show

Comments

Gao feng - Sept. 20, 2012, 5:25 a.m.
as we hold dst_entry before we call __ip6_del_rt,
so we should alse call dst_release not only return
-ENOENT when the rt6_info is ip6_null_entry.

and we already hold the dst entry, so I think it's
safe to call dst_release out of the write-read lock.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 net/ipv6/route.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
David Miller - Sept. 21, 2012, 5:16 p.m.
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 20 Sep 2012 13:25:34 +0800

> as we hold dst_entry before we call __ip6_del_rt,
> so we should alse call dst_release not only return
> -ENOENT when the rt6_info is ip6_null_entry.
> 
> and we already hold the dst entry, so I think it's
> safe to call dst_release out of the write-read lock.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

I cannot find a code path where we can actually end up
with an ip6_null_entry here and I'd much rather declare
this condition as a bug.

Patrick McHardy added this check back in 2006:

	commit 6c813a7297e3af4cd7c3458e09e9ee3d161c6830
	Author: Patrick McHardy <kaber@trash.net>
	Date:   Sun Aug 6 22:22:47 2006 -0700

	    [IPV6]: Fix crash in ip6_del_rt

But the ipv6 code has changed substantially since then.
--
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
Gao feng - Sept. 24, 2012, 4:32 a.m.
于 2012年09月22日 01:16, David Miller 写道:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Thu, 20 Sep 2012 13:25:34 +0800
> 
>> as we hold dst_entry before we call __ip6_del_rt,
>> so we should alse call dst_release not only return
>> -ENOENT when the rt6_info is ip6_null_entry.
>>
>> and we already hold the dst entry, so I think it's
>> safe to call dst_release out of the write-read lock.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> 
> I cannot find a code path where we can actually end up
> with an ip6_null_entry here and I'd much rather declare
> this condition as a bug.
> 
> Patrick McHardy added this check back in 2006:
> 
> 	commit 6c813a7297e3af4cd7c3458e09e9ee3d161c6830
> 	Author: Patrick McHardy <kaber@trash.net>
> 	Date:   Sun Aug 6 22:22:47 2006 -0700
> 
> 	    [IPV6]: Fix crash in ip6_del_rt
> 
> But the ipv6 code has changed substantially since then.

ip -6 route del ::/0 will end up with an ip6_null_entry.
so I think this checking and this patch is needed .

--
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
Gao feng - Oct. 4, 2012, 2:55 a.m.
于 2012年09月24日 12:32, Gao feng 写道:
> 于 2012年09月22日 01:16, David Miller 写道:
>> From: Gao feng <gaofeng@cn.fujitsu.com>
>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>
>>> as we hold dst_entry before we call __ip6_del_rt,
>>> so we should alse call dst_release not only return
>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>
>>> and we already hold the dst entry, so I think it's
>>> safe to call dst_release out of the write-read lock.
>>>
>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>
>> I cannot find a code path where we can actually end up
>> with an ip6_null_entry here and I'd much rather declare
>> this condition as a bug.
>>
>> Patrick McHardy added this check back in 2006:
>>
>> 	commit 6c813a7297e3af4cd7c3458e09e9ee3d161c6830
>> 	Author: Patrick McHardy <kaber@trash.net>
>> 	Date:   Sun Aug 6 22:22:47 2006 -0700
>>
>> 	    [IPV6]: Fix crash in ip6_del_rt
>>
>> But the ipv6 code has changed substantially since then.
> 
> ip -6 route del ::/0 will end up with an ip6_null_entry.
> so I think this checking and this patch is needed .
> 

Hi David

Can you apply this patch?
thanks!

--
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
David Miller - Oct. 4, 2012, 3:02 a.m.
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 04 Oct 2012 10:55:18 +0800

> 于 2012年09月24日 12:32, Gao feng 写道:
>> 于 2012年09月22日 01:16, David Miller 写道:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>>
>>>> as we hold dst_entry before we call __ip6_del_rt,
>>>> so we should alse call dst_release not only return
>>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>>
>>>> and we already hold the dst entry, so I think it's
>>>> safe to call dst_release out of the write-read lock.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
 ...
> Can you apply this patch?

Sorry, I've been really busy this week.  I will try to get to
this soon.

Thanks.
--
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
David Miller - Oct. 4, 2012, 8 p.m.
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Thu, 04 Oct 2012 10:55:18 +0800

> 于 2012年09月24日 12:32, Gao feng 写道:
>> 于 2012年09月22日 01:16, David Miller 写道:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Thu, 20 Sep 2012 13:25:34 +0800
>>>
>>>> as we hold dst_entry before we call __ip6_del_rt,
>>>> so we should alse call dst_release not only return
>>>> -ENOENT when the rt6_info is ip6_null_entry.
>>>>
>>>> and we already hold the dst entry, so I think it's
>>>> safe to call dst_release out of the write-read lock.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
 ...
> Can you apply this patch?

Applied and queued up for -stable, thanks.
--
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

Patch

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 854e401..46eff42 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1589,17 +1589,18 @@  static int __ip6_del_rt(struct rt6_info *rt, struct nl_info *info)
 	struct fib6_table *table;
 	struct net *net = dev_net(rt->dst.dev);
 
-	if (rt == net->ipv6.ip6_null_entry)
-		return -ENOENT;
+	if (rt == net->ipv6.ip6_null_entry) {
+		err = -ENOENT;
+		goto out;
+	}
 
 	table = rt->rt6i_table;
 	write_lock_bh(&table->tb6_lock);
-
 	err = fib6_del(rt, info);
-	dst_release(&rt->dst);
-
 	write_unlock_bh(&table->tb6_lock);
 
+out:
+	dst_release(&rt->dst);
 	return err;
 }