Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-25 Thread David Miller
From: Eric Biggers Date: Thu, 21 Feb 2019 14:13:56 -0800 > From: Eric Biggers > > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.") > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is > closed concurrently with fchownat(). However, it ignored that many

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-22 Thread Al Viro
On Fri, Feb 22, 2019 at 10:25:09AM -0800, Eric Dumazet wrote: > > > On 02/22/2019 09:57 AM, Eric Biggers wrote: > > > ->setattr() is called under inode_lock(), which __sock_release() also > > takes. So > > the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: > > close > >

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-22 Thread Eric Dumazet
On 02/22/2019 09:57 AM, Eric Biggers wrote: > ->setattr() is called under inode_lock(), which __sock_release() also takes. > So > the uses of sock->sk are serialized. See commit 6d8c50dcb029 ("socket: close > race condition between sock_close() and sockfs_setattr()"). Oh right, we added

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-22 Thread Cong Wang
On Thu, Feb 21, 2019 at 2:14 PM Eric Biggers wrote: > > From: Eric Biggers > > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.") > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is > closed concurrently with fchownat(). However, it ignored that many >

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-22 Thread Eric Biggers
Hi Eric, On Fri, Feb 22, 2019 at 09:45:35AM -0800, Eric Dumazet wrote: > > > On 02/21/2019 02:13 PM, Eric Biggers wrote: > > From: Eric Biggers > > > > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.") > > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-22 Thread Eric Dumazet
On 02/21/2019 02:13 PM, Eric Biggers wrote: > From: Eric Biggers > > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.") > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is > closed concurrently with fchownat(). However, it ignored that many > other

Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-21 Thread Al Viro
On Thu, Feb 21, 2019 at 02:13:56PM -0800, Eric Biggers wrote: > Rather than fixing all these and relying on every socket type to get > this right forever, just make __sock_release() set sock->sk to NULL > itself after calling proto_ops::release(). Is there any case when we would want

[PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()

2019-02-21 Thread Eric Biggers
From: Eric Biggers Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.") fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is closed concurrently with fchownat(). However, it ignored that many other proto_ops::release() methods don't set sock->sk to NULL and