Skip to content

Commit a2562d4

Browse files
dhowellsgregkh
authored andcommitted
rxrpc: Fix missing active use pinning of rxrpc_local object
[ Upstream commit 04d36d7 ] The introduction of a split between the reference count on rxrpc_local objects and the usage count didn't quite go far enough. A number of kernel work items need to make use of the socket to perform transmission. These also need to get an active count on the local object to prevent the socket from being closed. Fix this by getting the active count in those places. Also split out the raw active count get/put functions as these places tend to hold refs on the rxrpc_local object already, so getting and putting an extra object ref is just a waste of time. The problem can lead to symptoms like: BUG: kernel NULL pointer dereference, address: 0000000000000018 .. CPU: 2 PID: 818 Comm: kworker/u9:0 Not tainted 5.5.0-fscache+ #51 ... RIP: 0010:selinux_socket_sendmsg+0x5/0x13 ... Call Trace: security_socket_sendmsg+0x2c/0x3e sock_sendmsg+0x1a/0x46 rxrpc_send_keepalive+0x131/0x1ae rxrpc_peer_keepalive_worker+0x219/0x34b process_one_work+0x18e/0x271 worker_thread+0x1a3/0x247 kthread+0xe6/0xeb ret_from_fork+0x1f/0x30 Fixes: 730c5fd ("rxrpc: Fix local endpoint refcounting") Signed-off-by: David Howells <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent dba8533 commit a2562d4

File tree

5 files changed

+63
-40
lines changed

5 files changed

+63
-40
lines changed

net/rxrpc/af_rxrpc.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
196196
service_in_use:
197197
write_unlock(&local->services_lock);
198198
rxrpc_unuse_local(local);
199+
rxrpc_put_local(local);
199200
ret = -EADDRINUSE;
200201
error_unlock:
201202
release_sock(&rx->sk);
@@ -906,6 +907,7 @@ static int rxrpc_release_sock(struct sock *sk)
906907
rxrpc_purge_queue(&sk->sk_receive_queue);
907908

908909
rxrpc_unuse_local(rx->local);
910+
rxrpc_put_local(rx->local);
909911
rx->local = NULL;
910912
key_put(rx->key);
911913
rx->key = NULL;

net/rxrpc/ar-internal.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,16 @@ void rxrpc_unuse_local(struct rxrpc_local *);
10061006
void rxrpc_queue_local(struct rxrpc_local *);
10071007
void rxrpc_destroy_all_locals(struct rxrpc_net *);
10081008

1009+
static inline bool __rxrpc_unuse_local(struct rxrpc_local *local)
1010+
{
1011+
return atomic_dec_return(&local->active_users) == 0;
1012+
}
1013+
1014+
static inline bool __rxrpc_use_local(struct rxrpc_local *local)
1015+
{
1016+
return atomic_fetch_add_unless(&local->active_users, 1, 0) != 0;
1017+
}
1018+
10091019
/*
10101020
* misc.c
10111021
*/

net/rxrpc/conn_event.c

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -453,16 +453,12 @@ static void rxrpc_process_delayed_final_acks(struct rxrpc_connection *conn)
453453
/*
454454
* connection-level event processor
455455
*/
456-
void rxrpc_process_connection(struct work_struct *work)
456+
static void rxrpc_do_process_connection(struct rxrpc_connection *conn)
457457
{
458-
struct rxrpc_connection *conn =
459-
container_of(work, struct rxrpc_connection, processor);
460458
struct sk_buff *skb;
461459
u32 abort_code = RX_PROTOCOL_ERROR;
462460
int ret;
463461

464-
rxrpc_see_connection(conn);
465-
466462
if (test_and_clear_bit(RXRPC_CONN_EV_CHALLENGE, &conn->events))
467463
rxrpc_secure_connection(conn);
468464

@@ -490,18 +486,33 @@ void rxrpc_process_connection(struct work_struct *work)
490486
}
491487
}
492488

493-
out:
494-
rxrpc_put_connection(conn);
495-
_leave("");
496489
return;
497490

498491
requeue_and_leave:
499492
skb_queue_head(&conn->rx_queue, skb);
500-
goto out;
493+
return;
501494

502495
protocol_error:
503496
if (rxrpc_abort_connection(conn, ret, abort_code) < 0)
504497
goto requeue_and_leave;
505498
rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
506-
goto out;
499+
return;
500+
}
501+
502+
void rxrpc_process_connection(struct work_struct *work)
503+
{
504+
struct rxrpc_connection *conn =
505+
container_of(work, struct rxrpc_connection, processor);
506+
507+
rxrpc_see_connection(conn);
508+
509+
if (__rxrpc_use_local(conn->params.local)) {
510+
rxrpc_do_process_connection(conn);
511+
rxrpc_unuse_local(conn->params.local);
512+
}
513+
514+
rxrpc_put_connection(conn);
515+
_leave("");
516+
return;
507517
}
518+

net/rxrpc/local_object.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -387,14 +387,11 @@ void rxrpc_put_local(struct rxrpc_local *local)
387387
*/
388388
struct rxrpc_local *rxrpc_use_local(struct rxrpc_local *local)
389389
{
390-
unsigned int au;
391-
392390
local = rxrpc_get_local_maybe(local);
393391
if (!local)
394392
return NULL;
395393

396-
au = atomic_fetch_add_unless(&local->active_users, 1, 0);
397-
if (au == 0) {
394+
if (!__rxrpc_use_local(local)) {
398395
rxrpc_put_local(local);
399396
return NULL;
400397
}
@@ -408,14 +405,11 @@ struct rxrpc_local *rxrpc_use_local(struct rxrpc_local *local)
408405
*/
409406
void rxrpc_unuse_local(struct rxrpc_local *local)
410407
{
411-
unsigned int au;
412-
413408
if (local) {
414-
au = atomic_dec_return(&local->active_users);
415-
if (au == 0)
409+
if (__rxrpc_unuse_local(local)) {
410+
rxrpc_get_local(local);
416411
rxrpc_queue_local(local);
417-
else
418-
rxrpc_put_local(local);
412+
}
419413
}
420414
}
421415

@@ -472,7 +466,7 @@ static void rxrpc_local_processor(struct work_struct *work)
472466

473467
do {
474468
again = false;
475-
if (atomic_read(&local->active_users) == 0) {
469+
if (!__rxrpc_use_local(local)) {
476470
rxrpc_local_destroyer(local);
477471
break;
478472
}
@@ -486,6 +480,8 @@ static void rxrpc_local_processor(struct work_struct *work)
486480
rxrpc_process_local_events(local);
487481
again = true;
488482
}
483+
484+
__rxrpc_unuse_local(local);
489485
} while (again);
490486

491487
rxrpc_put_local(local);

net/rxrpc/peer_event.c

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -357,27 +357,31 @@ static void rxrpc_peer_keepalive_dispatch(struct rxrpc_net *rxnet,
357357
if (!rxrpc_get_peer_maybe(peer))
358358
continue;
359359

360-
spin_unlock_bh(&rxnet->peer_hash_lock);
361-
362-
keepalive_at = peer->last_tx_at + RXRPC_KEEPALIVE_TIME;
363-
slot = keepalive_at - base;
364-
_debug("%02x peer %u t=%d {%pISp}",
365-
cursor, peer->debug_id, slot, &peer->srx.transport);
360+
if (__rxrpc_use_local(peer->local)) {
361+
spin_unlock_bh(&rxnet->peer_hash_lock);
362+
363+
keepalive_at = peer->last_tx_at + RXRPC_KEEPALIVE_TIME;
364+
slot = keepalive_at - base;
365+
_debug("%02x peer %u t=%d {%pISp}",
366+
cursor, peer->debug_id, slot, &peer->srx.transport);
367+
368+
if (keepalive_at <= base ||
369+
keepalive_at > base + RXRPC_KEEPALIVE_TIME) {
370+
rxrpc_send_keepalive(peer);
371+
slot = RXRPC_KEEPALIVE_TIME;
372+
}
366373

367-
if (keepalive_at <= base ||
368-
keepalive_at > base + RXRPC_KEEPALIVE_TIME) {
369-
rxrpc_send_keepalive(peer);
370-
slot = RXRPC_KEEPALIVE_TIME;
374+
/* A transmission to this peer occurred since last we
375+
* examined it so put it into the appropriate future
376+
* bucket.
377+
*/
378+
slot += cursor;
379+
slot &= mask;
380+
spin_lock_bh(&rxnet->peer_hash_lock);
381+
list_add_tail(&peer->keepalive_link,
382+
&rxnet->peer_keepalive[slot & mask]);
383+
rxrpc_unuse_local(peer->local);
371384
}
372-
373-
/* A transmission to this peer occurred since last we examined
374-
* it so put it into the appropriate future bucket.
375-
*/
376-
slot += cursor;
377-
slot &= mask;
378-
spin_lock_bh(&rxnet->peer_hash_lock);
379-
list_add_tail(&peer->keepalive_link,
380-
&rxnet->peer_keepalive[slot & mask]);
381385
rxrpc_put_peer_locked(peer);
382386
}
383387

0 commit comments

Comments
 (0)