netdev - Re: [PATCH net] af_key: free SKBs under RCU protection

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]

Date:   Mon, 24 Sep 2018 12:46:07 -0600
From:   stranche@...eaurora.org
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     netdev@...r.kernel.org, steffen.klassert@...unet.com
Subject: Re: [PATCH net] af_key: free SKBs under RCU protection

On 2018-09-23 11:15, Eric Dumazet wrote:
> On 09/20/2018 12:25 PM, stranche@...eaurora.org wrote:
> 
>> Perhaps a cleaner solution here is to always clone the SKB in
>> pfkey_broadcast_one(). That will ensure that the two kfree_skb() calls
>> in pfkey_broadcast() will never be passed an SKB with sock_rfree() as
>> its destructor, and we can avoid this race condition.
> 
> Yes, this whole idea of avoiding the cloning is brain dead.
> 
> Better play safe and having a straightforward implementation.
> 
> I suggest something like this (I could not reproduce the bug with the
> syzkaller repro)
> 
> Note that I removed the sock_hold(sk)/sock_put(sk) pair as this is 
> useless.
> The only time GFP_KERNEL might be used is when the sk is already owned
> by the caller.
> 
> 
>  net/key/af_key.c |   40 +++++++++++++++-------------------------
>  1 file changed, 15 insertions(+), 25 deletions(-)

Hi Eric,

That patch works like a charm. Could you upload that as a formal patch?
Thanks for all your help with this.