netdev - Re: [PATCH net] net: ipv6: Do not duplicate DAD on link up

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 15:29:23 -0600
From:   David Ahern <dsahern@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH net] net: ipv6: Do not duplicate DAD on link up

On 5/3/17 3:22 PM, Cong Wang wrote:
>> Andrey's reproducer program runs in a very tight loop, calling
>> 'unshare -n' and then spawning 2 sets of 14 threads running random ioctl
>> calls. The relevant networking sequence:
>>
>> 1. New network namespace created via unshare -n
>> - ip6tnl0 device is created in down state
>>
>> 2. address added to ip6tnl0
>> - equivalent to ip -6 addr add dev ip6tnl0 fd00::bb/1
>> - DAD is started on the address and when it completes the host
>>   route is inserted into the FIB
>>
>> 3. ip6tnl0 is brought up
>> - the new fixup_permanent_addr function restarts DAD on the address
>>
>> 4. exit namespace
>> - teardown / cleanup sequence starts
>> - once in a blue moon, lo teardown appears to happen BEFORE teardown
>>   of ip6tunl0
> 
> Why is this possible? Loopback device should be always the first one to
> register therefore the last one to unregister.

Andrey's config builds in all of the devices. Once in a blue moon I
would see the host route get deleted b/c 'lo' was taken down.

> 
> Hmm? But addrconf_ifdown() is called when the device is going from UP
> to down, how does your patch prevent a duplicated DAD for a device which
> is going from DOWN to UP in step 3? This is very confusing.
> 

read the sequence:
1. ip6tnl0 is in the down state

2. address is added to ip6tnl0
   DAD starts, completes, inserts host route into FIB

3. ip6tnl0 is brought up
   DAD starts, completes, inserts host route into FIB

Between 2. and 3. lo is taken down.

Since DAD is done for an address in 2, it does not need to be repeated in 3.

The DAD in 3. was meant for addresses kept on an admin down. This patch
does that.