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

Message-ID: <9a3d8036-9f8c-6e8b-f16c-7c278c448db3@gmail.com>
Date:   Sun, 23 Sep 2018 10:15:41 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     stranche@...eaurora.org, 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 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(-)


View attachment "patch2712.txt" of type "text/plain" (2418 bytes)