On 2/27/17 12:34 PM, Eric Dumazet wrote:
> On Mon, 2017-02-27 at 11:07 -0800, Cong Wang wrote:
>> Andrey reported a NULL pointer deref bug in ipv6_route_ioctl()
>> -> ip6_route_del() -> __ip6_del_rt_siblings() code path. This is
>> because ip6_null_entry is returned in this path since ip6_null_entry
>> is kinda default for a ipv6 route table root node. Quote from
>> David Ahern:
>>
>> ip6_null_entry is the root of all ipv6 fib tables making it integrated
>> into the table ...
>>
>> We should ignore any attempt of trying to delete it, like we do in
>> __ip6_del_rt() path and several others.
>>
>> Reported-by: Andrey Konovalov <andreyk...@google.com>
>> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
>> ---
>> net/ipv6/route.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index f54f426..9da77e9 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -2169,10 +2169,13 @@ int ip6_del_rt(struct rt6_info *rt)
>> static int __ip6_del_rt_siblings(struct rt6_info *rt, struct fib6_config
>> *cfg)
>> {
>> struct nl_info *info = &cfg->fc_nlinfo;
>> + struct net *net = info->nl_net;
>> struct sk_buff *skb = NULL;
>> struct fib6_table *table;
>> int err;
>>
>> + if (rt == net->ipv6.ip6_null_entry)
>> + return -ENOENT;
>
> It looks the caller did a dst_hold(&rt->dst);
>
> So this new error path would leave a refcount leak.
>
> Note that I was not able to trigger the crash on old kernels, so it
> would be nice to get a precise idea of bug origin.
>
Cong: do you want to send a v2 catching the null entry in ip6_route_del
before the refcnt?
for (rt = fn->leaf; rt; rt = rt->dst.rt6_next) {
+ /* do not allow deletion of the null route */
+ if (rt == net->ipv6.ip6_null_entry)
+ continue;
Fixes: 0ae8133586ad net: ipv6: Allow shorthand delete of all nexthops in
multipath route
