svcrpc: avoid memory-corruption on pool shutdown
commita141a5eb3ab45131cb168e7a561d662722b43ec3
authorJ. Bruce Fields <bfields@redhat.com>
Tue, 29 Nov 2011 22:00:26 +0000 (29 17:00 -0500)
committerGreg Kroah-Hartman <gregkh@suse.de>
Thu, 26 Jan 2012 01:24:48 +0000 (25 17:24 -0800)
treef72da23ff947391cc5181842a34b39db81f1dc63
parent7df22768c0af8769d805f6db21144d71d91fe13d
svcrpc: avoid memory-corruption on pool shutdown

commit b4f36f88b3ee7cf26bf0be84e6c7fc15f84dcb71 upstream.

Socket callbacks use svc_xprt_enqueue() to add an xprt to a
pool->sp_sockets list.  In normal operation a server thread will later
come along and take the xprt off that list.  On shutdown, after all the
threads have exited, we instead manually walk the sv_tempsocks and
sv_permsocks lists to find all the xprt's and delete them.

So the sp_sockets lists don't really matter any more.  As a result,
we've mostly just ignored them and hoped they would go away.

Which has gotten us into trouble; witness for example ebc63e531cc6
"svcrpc: fix list-corrupting race on nfsd shutdown", the result of Ben
Greear noticing that a still-running svc_xprt_enqueue() could re-add an
xprt to an sp_sockets list just before it was deleted.  The fix was to
remove it from the list at the end of svc_delete_xprt().  But that only
made corruption less likely--I can see nothing that prevents a
svc_xprt_enqueue() from adding another xprt to the list at the same
moment that we're removing this xprt from the list.  In fact, despite
the earlier xpo_detach(), I don't even see what guarantees that
svc_xprt_enqueue() couldn't still be running on this xprt.

So, instead, note that svc_xprt_enqueue() essentially does:
lock sp_lock
if XPT_BUSY unset
add to sp_sockets
unlock sp_lock

So, if we do:

set XPT_BUSY on every xprt.
Empty every sp_sockets list, under the sp_socks locks.

Then we're left knowing that the sp_sockets lists are all empty and will
stay that way, since any svc_xprt_enqueue() will check XPT_BUSY under
the sp_lock and see it set.

And *then* we can continue deleting the xprt's.

(Thanks to Jeff Layton for being correctly suspicious of this code....)

Cc: Ben Greear <greearb@candelatech.com>
Cc: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
net/sunrpc/svc.c
net/sunrpc/svc_xprt.c