Skip to content

Commit 7926681

Browse files
dhowellsgregkh
authored andcommitted
rxrpc: Fix lack of conn cleanup when local endpoint is cleaned up [ver #2]
[ Upstream commit d12040b ] When a local endpoint is ceases to be in use, such as when the kafs module is unloaded, the kernel will emit an assertion failure if there are any outstanding client connections: rxrpc: Assertion failed ------------[ cut here ]------------ kernel BUG at net/rxrpc/local_object.c:433! and even beyond that, will evince other oopses if there are service connections still present. Fix this by: (1) Removing the triggering of connection reaping when an rxrpc socket is released. These don't actually clean up the connections anyway - and further, the local endpoint may still be in use through another socket. (2) Mark the local endpoint as dead when we start the process of tearing it down. (3) When destroying a local endpoint, strip all of its client connections from the idle list and discard the ref on each that the list was holding. (4) When destroying a local endpoint, call the service connection reaper directly (rather than through a workqueue) to immediately kill off all outstanding service connections. (5) Make the service connection reaper reap connections for which the local endpoint is marked dead. Only after destroying the connections can we close the socket lest we get an oops in a workqueue that's looking at a connection or a peer. Fixes: 3d18cbb ("rxrpc: Fix conn expiry timers") Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Marc Dionne <marc.dionne@auristor.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 5571688 commit 7926681

5 files changed

Lines changed: 50 additions & 5 deletions

File tree

net/rxrpc/af_rxrpc.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -869,7 +869,6 @@ static void rxrpc_sock_destructor(struct sock *sk)
869869
static int rxrpc_release_sock(struct sock *sk)
870870
{
871871
struct rxrpc_sock *rx = rxrpc_sk(sk);
872-
struct rxrpc_net *rxnet = rxrpc_net(sock_net(&rx->sk));
873872

874873
_enter("%p{%d,%d}", sk, sk->sk_state, refcount_read(&sk->sk_refcnt));
875874

@@ -905,8 +904,6 @@ static int rxrpc_release_sock(struct sock *sk)
905904
rxrpc_release_calls_on_socket(rx);
906905
flush_workqueue(rxrpc_workqueue);
907906
rxrpc_purge_queue(&sk->sk_receive_queue);
908-
rxrpc_queue_work(&rxnet->service_conn_reaper);
909-
rxrpc_queue_work(&rxnet->client_conn_reaper);
910907

911908
rxrpc_unuse_local(rx->local);
912909
rx->local = NULL;

net/rxrpc/ar-internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -903,6 +903,7 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *);
903903
void rxrpc_put_client_conn(struct rxrpc_connection *);
904904
void rxrpc_discard_expired_client_conns(struct work_struct *);
905905
void rxrpc_destroy_all_client_connections(struct rxrpc_net *);
906+
void rxrpc_clean_up_local_conns(struct rxrpc_local *);
906907

907908
/*
908909
* conn_event.c

net/rxrpc/conn_client.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,3 +1166,47 @@ void rxrpc_destroy_all_client_connections(struct rxrpc_net *rxnet)
11661166

11671167
_leave("");
11681168
}
1169+
1170+
/*
1171+
* Clean up the client connections on a local endpoint.
1172+
*/
1173+
void rxrpc_clean_up_local_conns(struct rxrpc_local *local)
1174+
{
1175+
struct rxrpc_connection *conn, *tmp;
1176+
struct rxrpc_net *rxnet = local->rxnet;
1177+
unsigned int nr_active;
1178+
LIST_HEAD(graveyard);
1179+
1180+
_enter("");
1181+
1182+
spin_lock(&rxnet->client_conn_cache_lock);
1183+
nr_active = rxnet->nr_active_client_conns;
1184+
1185+
list_for_each_entry_safe(conn, tmp, &rxnet->idle_client_conns,
1186+
cache_link) {
1187+
if (conn->params.local == local) {
1188+
ASSERTCMP(conn->cache_state, ==, RXRPC_CONN_CLIENT_IDLE);
1189+
1190+
trace_rxrpc_client(conn, -1, rxrpc_client_discard);
1191+
if (!test_and_clear_bit(RXRPC_CONN_EXPOSED, &conn->flags))
1192+
BUG();
1193+
conn->cache_state = RXRPC_CONN_CLIENT_INACTIVE;
1194+
list_move(&conn->cache_link, &graveyard);
1195+
nr_active--;
1196+
}
1197+
}
1198+
1199+
rxnet->nr_active_client_conns = nr_active;
1200+
spin_unlock(&rxnet->client_conn_cache_lock);
1201+
ASSERTCMP(nr_active, >=, 0);
1202+
1203+
while (!list_empty(&graveyard)) {
1204+
conn = list_entry(graveyard.next,
1205+
struct rxrpc_connection, cache_link);
1206+
list_del_init(&conn->cache_link);
1207+
1208+
rxrpc_put_connection(conn);
1209+
}
1210+
1211+
_leave(" [culled]");
1212+
}

net/rxrpc/conn_object.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ void rxrpc_service_connection_reaper(struct work_struct *work)
401401
if (conn->state == RXRPC_CONN_SERVICE_PREALLOC)
402402
continue;
403403

404-
if (rxnet->live) {
404+
if (rxnet->live && !conn->params.local->dead) {
405405
idle_timestamp = READ_ONCE(conn->idle_timestamp);
406406
expire_at = idle_timestamp + rxrpc_connection_expiry * HZ;
407407
if (conn->params.local->service_closed)

net/rxrpc/local_object.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,11 +430,14 @@ static void rxrpc_local_destroyer(struct rxrpc_local *local)
430430

431431
_enter("%d", local->debug_id);
432432

433+
local->dead = true;
434+
433435
mutex_lock(&rxnet->local_mutex);
434436
list_del_init(&local->link);
435437
mutex_unlock(&rxnet->local_mutex);
436438

437-
ASSERT(RB_EMPTY_ROOT(&local->client_conns));
439+
rxrpc_clean_up_local_conns(local);
440+
rxrpc_service_connection_reaper(&rxnet->service_conn_reaper);
438441
ASSERT(!local->service);
439442

440443
if (socket) {

0 commit comments

Comments
 (0)