lkml.org 
[lkml]   [2019]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
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 sock->sk non-NULL when
sock->sk->sk_socket is NULL?

IOW, why not make sock_orphan() take care of that, at the same
time we dissociate sock from socket? After all, on the other end
of things we have both sock_graft() and sock_init_data() set both
sock->sk and sk->sk_socket...

Looking at the assignments to sock->sk, I see
* atalk_release(), bcm_release(), raw_release(), pfkey_release(),
pppol2tp_release(), packet_release(), smc_release(), xsk_release() -
directly after sock_orphan()
* rxrpc_release() - directly *before* sock_orphan()

* ax25_release() - somewhat after sock_orphan(); no idea if anything
done in between needs it still set. Doesn't looks like there could
be - readers of sock->sk in there are all in methods that can't
overlap with ->release().
* kcm_release() - similar.
* rds_release() - similar.
* tipc_release() - similar.
* unix_accept() - similar, and there I'm fairly sure it could be
done right after sock_orphan()
* netrom_release() - similar. BTW, why the hell does nr_accept()
check for NULL sock->sk, while other methods (callable for exact
same sockets) assume that it's non-NULL? AFAICS, the check in
nr_accept() is pointless - nr_create() can leave sock->sk NULL
only if it fails, in which case it's going to be immediately
hit by ->release() and freed by iput() in sock_release().
* rose_release() - similar, complete with ->accept() oddity.

* caif_release() - done *before* sock_orphan(), with other bit of
the latter duplicated just prior. NFI why; it messes with
debugfs, so there's a whole kettle of copulating slugs involved ;-/

* ieee802154_sock_release(), inet_release(), pn_socket_release() -
done before calliing into protocol's ->close(), which ends up
calling sock_orphan(), either directly or via sk_common_release().
I suspect that we would be find with zeroing sock->sk delayed
until sock_orphan() in all of those, but that needs more RTFS
than I want to go into at the moment.

* netlink_release() - somewhat after sock_orphan(); not sure,
calls of ->netlink_unbind() done inbetween might want it still
set.

* qrtr_release() - lacks sock_orphan(), might be broken

* vsock_releae() - NFI.

* vcc_create() - clears sock->sk in the very beginning; AFAICS
that's pointless.

\
 
 \ /
  Last update: 2019-02-22 03:52    [W:0.064 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site