netdev - Re: [PATCH] net: ipv6: Fix warning of freeing alive inet6 address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 3 May 2017 16:58:36 +0200
From:   Andrey Konovalov <andreyknvl@...gle.com>
To:     Mike Manning <mmanning@...cade.com>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: ipv6: Fix warning of freeing alive inet6 address

On Wed, May 3, 2017 at 4:16 PM, Mike Manning <mmanning@...cade.com> wrote:
> On reflection, please put this on hold subject to testing with syzkaller. I have not had a repro of the issue and so the fix even though harmless may not be effective.

Hi Mike,

I didn't see your patch, you think you might have forgotten to add me
to recipients.

Did you try building the kernel with the provided config and then
running the reproducer?

Would you like me to test if this patch fixes the warning?

Thanks!

>
> Thanks
> Mike
>
> On 02/05/17 19:30, Mike Manning wrote:
>> While this is not reproducible manually, Andrey's syzkaller program hit
>> the warning "IPv6: Freeing alive inet6 address" with this part trace:
>>
>> inet6_ifa_finish_destroy+0x12e/0x190 c:894
>> in6_ifa_put ./include/net/addrconf.h:330
>> addrconf_dad_work+0x4e9/0x1040 net/ipv6/addrconf.c:3963
>>
>> The fix is to call in6_ifa_put() for the inet6_ifaddr before rather
>> than after calling addrconf_ifdown(), as the latter may remove it from
>> the address hash table.
>>
>> Fixes: 85b51b12115c ("net: ipv6: Remove addresses for failures with strict DAD")
>> Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
>> Signed-off-by: Mike Manning <mmanning@...cade.com>
>> ---
>>  net/ipv6/addrconf.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 80ce478..361993a 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -3902,8 +3902,11 @@ static void addrconf_dad_work(struct work_struct *w)
>>       } else if (action == DAD_ABORT) {
>>               in6_ifa_hold(ifp);
>>               addrconf_dad_stop(ifp, 1);
>> -             if (disable_ipv6)
>> +             if (disable_ipv6) {
>> +                     in6_ifa_put(ifp);
>>                       addrconf_ifdown(idev->dev, 0);
>> +                     goto unlock;
>> +             }
>>               goto out;
>>       }
>>
>> @@ -3950,6 +3953,7 @@ static void addrconf_dad_work(struct work_struct *w)
>>                     ifp->dad_nonce);
>>  out:
>>       in6_ifa_put(ifp);
>> +unlock:
>>       rtnl_unlock();
>>  }
>>
>>
>