scoutfs: release server conn under mutex

I was rarely seeing null derefs during unmount.  The per-mount listening
scoutfs_server_func() was seeing null sock->ops as it called
kernel_sock_shutdown() to shutdown the connected client sockets.
sock_release() sets the ops to null.  We're not supposed to use a socket
after we call it.

The per-connection scoutfs_server_recv_func() calls sock_release() as it
tears down its connection.  But it does this before it removes the
connection from the listener's list.  There's a brief window where the
connection's socket has been released but is still visible on the list.
If the listener tries to shutdown during this time it will crash.

Hitting this window depends on scheduling races during unmount.  The
unmount path has the client close its connection to the server then the
server closes all its connected clients.  If the local mount is the
server then it will have recv work see an error as the client
disconnects and it will be racing to shut down the connection with the
listening thread during unmount.

I think I only saw this in my guests because they're running slower
debug kernels on my slower laptop.  The window of vulnerability while
the released socket is on the list is longer.

The fix is to release the socket while we hold the mutex and are
removing the connection from the list.  A released socket is never
visible on the list.

While we're at it don't use list_for_each_entry_safe() to iterate over
the connection list.  We're not modifying it.  This is an lingering
artifact from previous versions of the server code.

Signed-off-by: Zach Brown <zab@versity.com>
This commit is contained in:
Zach Brown
2018-02-22 14:27:01 -08:00
committed by Zach Brown
parent e31e828aff
commit c76c6582f0
+2 -4
View File
@@ -867,10 +867,9 @@ out:
destroy_workqueue(req_wq);
}
sock_release(conn->sock);
/* process_one_work explicitly allows freeing work in its func */
mutex_lock(&server->mutex);
sock_release(conn->sock);
list_del_init(&conn->head);
kfree(conn);
smp_mb();
@@ -920,7 +919,6 @@ static void scoutfs_server_func(struct work_struct *work)
struct socket *sock = NULL;
struct scoutfs_lock *lock = NULL;
struct server_connection *conn;
struct server_connection *conn_tmp;
struct pending_seq *ps;
struct pending_seq *ps_tmp;
DECLARE_WAIT_QUEUE_HEAD(waitq);
@@ -1043,7 +1041,7 @@ static void scoutfs_server_func(struct work_struct *work)
/* shutdown send and recv on all accepted sockets */
mutex_lock(&server->mutex);
list_for_each_entry_safe(conn, conn_tmp, &conn_list, head)
list_for_each_entry(conn, &conn_list, head)
kernel_sock_shutdown(conn->sock, SHUT_RDWR);
mutex_unlock(&server->mutex);