netdev - Re: [PATCH v2 net 3/3] wireguard: send: account for mtu=0 devices

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]

Message-ID: <1b132351-d4a7-851c-ac98-0a48c8d90797@gmail.com>
Date:   Fri, 14 Feb 2020 14:30:34 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Eric Dumazet <eric.dumazet@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net 3/3] wireguard: send: account for mtu=0 devices



On 2/14/20 1:57 PM, Jason A. Donenfeld wrote:

> 
> Thanks, I appreciate your scrutiny here. Right again, you are. It
> looks like that was added in 2017 after observing the pattern in other
> drivers and seeing the documentation comment, "Wait for packets
> currently being received to be done." That sounds like an important
> thing to do before tearing down a socket. But here it makes no sense
> at all, since synchronize_net() is just a wrapper around
> synchronize_rcu() (and sometimes _expedited). And here, the
> synchronize_rcu() usage makes sense to have, since this is as boring
> of an rcu pattern as can be:
> 
> mutex_lock()
> old = rcu_dereference_protected(x->y)
> rcu_assign(x->y, new)
> mutex_unlock()
> synchronize_rcu()
> free_it(old)
> 
> Straight out of the documentation. Having the extra synchronize_net()
> in there adds nothing at all. I'll send a v3 of this 5.6-rc2 cleanup
> series containing that removal.
> 

Also note that UDP sockets have SOCK_RCU_FREE flag set, so core
networking also respect one RCU grace period before freeing them.

It is possible that no extra synchronize_{net|rcu}() call is needed,
but this is left as an exercise for future kernels :)