Skip to content

Commit 608aecd

Browse files
dhowellsdavem330
authored andcommitted
rxrpc: Fix locking issues in rxrpc_put_peer_locked()
Now that rxrpc_put_local() may call kthread_stop(), it can't be called under spinlock as it might sleep. This can cause a problem in the peer keepalive code in rxrpc as it tries to avoid dropping the peer_hash_lock from the point it needs to re-add peer->keepalive_link to going round the loop again in rxrpc_peer_keepalive_dispatch(). Fix this by just dropping the lock when we don't need it and accepting that we'll have to take it again. This code is only called about every 20s for each peer, so not very often. This allows rxrpc_put_peer_unlocked() to be removed also. If triggered, this bug produces an oops like the following, as reproduced by a syzbot reproducer for a different oops[1]: BUG: sleeping function called from invalid context at kernel/sched/completion.c:101 ... RCU nest depth: 0, expected: 0 3 locks held by kworker/u9:0/50: #0: ffff88810e74a138 ((wq_completion)krxrpcd){+.+.}-{0:0}, at: process_one_work+0x294/0x636 #1: ffff8881013a7e20 ((work_completion)(&rxnet->peer_keepalive_work)){+.+.}-{0:0}, at: process_one_work+0x294/0x636 #2: ffff88817d366390 (&rxnet->peer_hash_lock){+.+.}-{2:2}, at: rxrpc_peer_keepalive_dispatch+0x2bd/0x35f ... Call Trace: <TASK> dump_stack_lvl+0x4c/0x5f __might_resched+0x2cf/0x2f2 __wait_for_common+0x87/0x1e8 kthread_stop+0x14d/0x255 rxrpc_peer_keepalive_dispatch+0x333/0x35f rxrpc_peer_keepalive_worker+0x2e9/0x449 process_one_work+0x3c1/0x636 worker_thread+0x25f/0x359 kthread+0x1a6/0x1b5 ret_from_fork+0x1f/0x30 Fixes: a275da6 ("rxrpc: Create a per-local endpoint receive queue and I/O thread") Signed-off-by: David Howells <[email protected]> cc: Marc Dionne <[email protected]> cc: [email protected] Link: https://lore.kernel.org/r/[email protected]/ [1] Signed-off-by: David S. Miller <[email protected]>
1 parent 8fbcc83 commit 608aecd

File tree

3 files changed

+7
-23
lines changed

3 files changed

+7
-23
lines changed

net/rxrpc/ar-internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,6 @@ void rxrpc_destroy_all_peers(struct rxrpc_net *);
10731073
struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *, enum rxrpc_peer_trace);
10741074
struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *, enum rxrpc_peer_trace);
10751075
void rxrpc_put_peer(struct rxrpc_peer *, enum rxrpc_peer_trace);
1076-
void rxrpc_put_peer_locked(struct rxrpc_peer *, enum rxrpc_peer_trace);
10771076

10781077
/*
10791078
* proc.c

net/rxrpc/peer_event.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
235235
struct rxrpc_peer *peer;
236236
const u8 mask = ARRAY_SIZE(rxnet->peer_keepalive) - 1;
237237
time64_t keepalive_at;
238+
bool use;
238239
int slot;
239240

240241
spin_lock(&rxnet->peer_hash_lock);
@@ -247,9 +248,10 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
247248
if (!rxrpc_get_peer_maybe(peer, rxrpc_peer_get_keepalive))
248249
continue;
249250

250-
if (__rxrpc_use_local(peer->local, rxrpc_local_use_peer_keepalive)) {
251-
spin_unlock(&rxnet->peer_hash_lock);
251+
use = __rxrpc_use_local(peer->local, rxrpc_local_use_peer_keepalive);
252+
spin_unlock(&rxnet->peer_hash_lock);
252253

254+
if (use) {
253255
keepalive_at = peer->last_tx_at + RXRPC_KEEPALIVE_TIME;
254256
slot = keepalive_at - base;
255257
_debug("%02x peer %u t=%d {%pISp}",
@@ -270,9 +272,11 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
270272
spin_lock(&rxnet->peer_hash_lock);
271273
list_add_tail(&peer->keepalive_link,
272274
&rxnet->peer_keepalive[slot & mask]);
275+
spin_unlock(&rxnet->peer_hash_lock);
273276
rxrpc_unuse_local(peer->local, rxrpc_local_unuse_peer_keepalive);
274277
}
275-
rxrpc_put_peer_locked(peer, rxrpc_peer_put_keepalive);
278+
rxrpc_put_peer(peer, rxrpc_peer_put_keepalive);
279+
spin_lock(&rxnet->peer_hash_lock);
276280
}
277281

278282
spin_unlock(&rxnet->peer_hash_lock);

net/rxrpc/peer_object.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -438,25 +438,6 @@ void rxrpc_put_peer(struct rxrpc_peer *peer, enum rxrpc_peer_trace why)
438438
}
439439
}
440440

441-
/*
442-
* Drop a ref on a peer record where the caller already holds the
443-
* peer_hash_lock.
444-
*/
445-
void rxrpc_put_peer_locked(struct rxrpc_peer *peer, enum rxrpc_peer_trace why)
446-
{
447-
unsigned int debug_id = peer->debug_id;
448-
bool dead;
449-
int r;
450-
451-
dead = __refcount_dec_and_test(&peer->ref, &r);
452-
trace_rxrpc_peer(debug_id, r - 1, why);
453-
if (dead) {
454-
hash_del_rcu(&peer->hash_link);
455-
list_del_init(&peer->keepalive_link);
456-
rxrpc_free_peer(peer);
457-
}
458-
}
459-
460441
/*
461442
* Make sure all peer records have been discarded.
462443
*/

0 commit comments

Comments
 (0)