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

2018-09-24 Thread stranche
On 2018-09-23 11:15, Eric Dumazet wrote: On 09/20/2018 12:25 PM, stran...@codeaurora.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()

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

2018-09-23 Thread Eric Dumazet
On 09/20/2018 12:25 PM, stran...@codeaurora.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 avo

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

2018-09-21 Thread stranche
On 2018-09-21 11:40, Eric Dumazet wrote: On 09/21/2018 10:09 AM, stran...@codeaurora.org wrote: I also tried reverting 7f6b9dbd5afb ("af_key: locking change") and running the test there and I still see the crash, so it doesn't seem to be an RCU specific issue. Is there anything else that cou

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

2018-09-21 Thread Eric Dumazet
On 09/21/2018 10:09 AM, stran...@codeaurora.org wrote: > I also tried reverting 7f6b9dbd5afb ("af_key: locking change") and running the > test there and I still see the crash, so it doesn't seem to be an RCU specific > issue. > > Is there anything else that could be causing this? What about y

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

2018-09-21 Thread stranche
As long as one skb has sock_rfree has its destructor, the socket attached to this skb can not be released. There is no race here. Note that skb_clone() does not propagate the destructor. The issue here is that in the rcu lookup, we can find a socket that has been dismantled, with a 0 refcou

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

2018-09-20 Thread Eric Dumazet
On 09/20/2018 03:10 PM, Eric Dumazet wrote: > > > On 09/20/2018 12:25 PM, stran...@codeaurora.org wrote: >>> >>> I do not believe the changelog or the patch makes sense. >>> >>> Having skb still referencing a socket prevents this socket being released. >>> >>> If you think about it, what would

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

2018-09-20 Thread Eric Dumazet
On 09/20/2018 12:25 PM, stran...@codeaurora.org wrote: >> >> I do not believe the changelog or the patch makes sense. >> >> Having skb still referencing a socket prevents this socket being released. >> >> If you think about it, what would prevent the freeing happening >> _before_ the rcu_read_lo

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

2018-09-20 Thread stranche
I do not believe the changelog or the patch makes sense. Having skb still referencing a socket prevents this socket being released. If you think about it, what would prevent the freeing happening _before_ the rcu_read_lock() in pfkey_broadcast() ? Maybe the correct fix is that pfkey_broadcas

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

2018-09-20 Thread Eric Dumazet
On 09/19/2018 05:18 PM, Sean Tranchetti wrote: > pfkey_broadcast() can make calls to pfkey_broadcast_one() which > will clone or copy the passed in SKB. This new SKB will be assigned > the sock_rfree() function as its destructor, which requires that > the socket reference the SKB contains is val

[PATCH net] af_key: free SKBs under RCU protection

2018-09-19 Thread Sean Tranchetti
pfkey_broadcast() can make calls to pfkey_broadcast_one() which will clone or copy the passed in SKB. This new SKB will be assigned the sock_rfree() function as its destructor, which requires that the socket reference the SKB contains is valid when the SKB is freed. If this SKB is ever passed to p