On Mon, Feb 27, 2017 at 12:34 PM, Eric Dumazet <[email protected]> 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 <[email protected]>
>> Signed-off-by: Cong Wang <[email protected]>
>> ---
>> 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.
Interesting, this error path is not new for __ip6_del_rt_siblings()
so the leak was already there before mine, but you are probably
right we have a leak here.
I will send a separate patch to address this 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.
Right, I miss:
Fixes: 0ae8133586ad ("net: ipv6: Allow shorthand delete of all
nexthops in multipath route")
Thanks!
