netdev - Re: [PATCH v2] ipv4: synchronize bind() with RTM

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] [day] [month] [year] [list]

Message-ID: <4CC08DFE.7070400@iki.fi>
Date:	Thu, 21 Oct 2010 22:01:18 +0300
From:	Timo Teräs <timo.teras@....fi>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v2] ipv4: synchronize bind() with RTM_NEWADDR notifications

On 10/21/2010 05:10 PM, Eric Dumazet wrote:
> Le jeudi 21 octobre 2010 à 16:06 +0300, Timo Teräs a écrit :
>> Otherwise we have race condition to user land:
>>  1. process A: changes IP address
>>  2. process A: kernel sends RTM_NEWADDR (and schedules out)
>>  3. process B: gets notification
>>  4. process B: tries to bind() to new IP, but fails with EADDRNOTAVAIL
>>       because FIB is not yet updated and inet_addr_type() in inet_bind()
>>       does not recognize the IP as local
>>  5. process A: calls inetaddr_chain notifiers which updates FIB
>>
>> Fix the error path to synchronize with configuration changes and retry
>> the address type check.
>>
>> IPv6 side seems to handle the notifications properly: bind() immediately
>> after RTM_NEWADDR succeeds as expected.  This is because ipv6_chk_addr()
>> uses inet6_addr_lst which is updated before address notification.
>>
>> Signed-off-by: Timo Teräs <timo.teras@....fi>
>> ---
>> Since there was no reply to my question if this is ok, I interpreted it
>> as "maybe". So here's the code for review. Hopefully this helps determining
>> if this is an acceptable fix.
> 
> Just say : no
> 
> Really Timo, this problem must get another fix.
> 
> I understand you need an urgent fix, you can use your patch in the
> meantime, of course ;)

Fine. I figured you might feel this way. My final idea to fix this, is
to modify inet_addr_type() do the address type check using the ifa
lists. This probably involves making ifa lists rcu (did not take close
look on how ifa lists work in current ipv4 code). This is basically how
ipv6 side works. I'm not too sure how to go on with this, so I'll wait
up until someone more qualified gets the time to look at this.

I think for my immediate needs I might be able to get away with using
the kludge patch, enabling non-local binding or fixing my userland code
to listen RTM_NEWROUTE/RTN_LOCAL events (though, IPv6 link-local
addresses are special and would need RTM_NEWADDR handling still to get
the real device's ifindex).

Btw. why do IPv6 RTN_LOCAL routes have loopback interface as dst instead
of the real device? IPv4 RTN_LOCAL routes seem to have the real device
so this looks inconsistent at first sight. I guess IPv6 requires this
for a bunch of other things. This way it's just not really possible to
get link-local IPv6 routes.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html