Skip to content

Commit e5799bd

Browse files
committed
rxrpc: Fix call timer start racing with call destruction
jira LE-1907 Rebuild_History Non-Buildable kernel-rt-5.14.0-284.30.1.rt14.315.el9_2 commit-author David Howells <[email protected]> commit 4a7f62f The rxrpc_call struct has a timer used to handle various timed events relating to a call. This timer can get started from the packet input routines that are run in softirq mode with just the RCU read lock held. Unfortunately, because only the RCU read lock is held - and neither ref or other lock is taken - the call can start getting destroyed at the same time a packet comes in addressed to that call. This causes the timer - which was already stopped - to get restarted. Later, the timer dispatch code may then oops if the timer got deallocated first. Fix this by trying to take a ref on the rxrpc_call struct and, if successful, passing that ref along to the timer. If the timer was already running, the ref is discarded. The timer completion routine can then pass the ref along to the call's work item when it queues it. If the timer or work item where already queued/running, the extra ref is discarded. Fixes: a158bdd ("rxrpc: Fix call timeouts") Reported-by: Marc Dionne <[email protected]> Signed-off-by: David Howells <[email protected]> Reviewed-by: Marc Dionne <[email protected]> Tested-by: Marc Dionne <[email protected]> cc: [email protected] Link: http://lists.infradead.org/pipermail/linux-afs/2022-March/005073.html Link: https://lore.kernel.org/r/164865115696.2943015.11097991776647323586.stgit@warthog.procyon.org.uk Signed-off-by: Paolo Abeni <[email protected]> (cherry picked from commit 4a7f62f) Signed-off-by: Jonathan Maple <[email protected]>
1 parent 2029335 commit e5799bd

File tree

4 files changed

+50
-15
lines changed

4 files changed

+50
-15
lines changed

include/trace/events/rxrpc.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,15 @@ enum rxrpc_call_trace {
8383
rxrpc_call_error,
8484
rxrpc_call_got,
8585
rxrpc_call_got_kernel,
86+
rxrpc_call_got_timer,
8687
rxrpc_call_got_userid,
8788
rxrpc_call_new_client,
8889
rxrpc_call_new_service,
8990
rxrpc_call_put,
9091
rxrpc_call_put_kernel,
9192
rxrpc_call_put_noqueue,
93+
rxrpc_call_put_notimer,
94+
rxrpc_call_put_timer,
9295
rxrpc_call_put_userid,
9396
rxrpc_call_queued,
9497
rxrpc_call_queued_ref,
@@ -278,12 +281,15 @@ enum rxrpc_tx_point {
278281
EM(rxrpc_call_error, "*E*") \
279282
EM(rxrpc_call_got, "GOT") \
280283
EM(rxrpc_call_got_kernel, "Gke") \
284+
EM(rxrpc_call_got_timer, "GTM") \
281285
EM(rxrpc_call_got_userid, "Gus") \
282286
EM(rxrpc_call_new_client, "NWc") \
283287
EM(rxrpc_call_new_service, "NWs") \
284288
EM(rxrpc_call_put, "PUT") \
285289
EM(rxrpc_call_put_kernel, "Pke") \
286-
EM(rxrpc_call_put_noqueue, "PNQ") \
290+
EM(rxrpc_call_put_noqueue, "PnQ") \
291+
EM(rxrpc_call_put_notimer, "PnT") \
292+
EM(rxrpc_call_put_timer, "PTM") \
287293
EM(rxrpc_call_put_userid, "Pus") \
288294
EM(rxrpc_call_queued, "QUE") \
289295
EM(rxrpc_call_queued_ref, "QUR") \

net/rxrpc/ar-internal.h

+7-8
Original file line numberDiff line numberDiff line change
@@ -777,14 +777,12 @@ void rxrpc_propose_ACK(struct rxrpc_call *, u8, u32, bool, bool,
777777
enum rxrpc_propose_ack_trace);
778778
void rxrpc_process_call(struct work_struct *);
779779

780-
static inline void rxrpc_reduce_call_timer(struct rxrpc_call *call,
781-
unsigned long expire_at,
782-
unsigned long now,
783-
enum rxrpc_timer_trace why)
784-
{
785-
trace_rxrpc_timer(call, why, now);
786-
timer_reduce(&call->timer, expire_at);
787-
}
780+
void rxrpc_reduce_call_timer(struct rxrpc_call *call,
781+
unsigned long expire_at,
782+
unsigned long now,
783+
enum rxrpc_timer_trace why);
784+
785+
void rxrpc_delete_call_timer(struct rxrpc_call *call);
788786

789787
/*
790788
* call_object.c
@@ -808,6 +806,7 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *);
808806
bool __rxrpc_queue_call(struct rxrpc_call *);
809807
bool rxrpc_queue_call(struct rxrpc_call *);
810808
void rxrpc_see_call(struct rxrpc_call *);
809+
bool rxrpc_try_get_call(struct rxrpc_call *call, enum rxrpc_call_trace op);
811810
void rxrpc_get_call(struct rxrpc_call *, enum rxrpc_call_trace);
812811
void rxrpc_put_call(struct rxrpc_call *, enum rxrpc_call_trace);
813812
void rxrpc_cleanup_call(struct rxrpc_call *);

net/rxrpc/call_event.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ void rxrpc_process_call(struct work_struct *work)
310310
}
311311

312312
if (call->state == RXRPC_CALL_COMPLETE) {
313-
del_timer_sync(&call->timer);
313+
rxrpc_delete_call_timer(call);
314314
goto out_put;
315315
}
316316

net/rxrpc/call_object.c

+35-5
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,30 @@ static void rxrpc_call_timer_expired(struct timer_list *t)
5353

5454
if (call->state < RXRPC_CALL_COMPLETE) {
5555
trace_rxrpc_timer(call, rxrpc_timer_expired, jiffies);
56-
rxrpc_queue_call(call);
56+
__rxrpc_queue_call(call);
57+
} else {
58+
rxrpc_put_call(call, rxrpc_call_put);
59+
}
60+
}
61+
62+
void rxrpc_reduce_call_timer(struct rxrpc_call *call,
63+
unsigned long expire_at,
64+
unsigned long now,
65+
enum rxrpc_timer_trace why)
66+
{
67+
if (rxrpc_try_get_call(call, rxrpc_call_got_timer)) {
68+
trace_rxrpc_timer(call, why, now);
69+
if (timer_reduce(&call->timer, expire_at))
70+
rxrpc_put_call(call, rxrpc_call_put_notimer);
5771
}
5872
}
5973

74+
void rxrpc_delete_call_timer(struct rxrpc_call *call)
75+
{
76+
if (del_timer_sync(&call->timer))
77+
rxrpc_put_call(call, rxrpc_call_put_timer);
78+
}
79+
6080
static struct lock_class_key rxrpc_call_user_mutex_lock_class_key;
6181

6282
/*
@@ -463,6 +483,17 @@ void rxrpc_see_call(struct rxrpc_call *call)
463483
}
464484
}
465485

486+
bool rxrpc_try_get_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
487+
{
488+
const void *here = __builtin_return_address(0);
489+
int n = atomic_fetch_add_unless(&call->usage, 1, 0);
490+
491+
if (n == 0)
492+
return false;
493+
trace_rxrpc_call(call->debug_id, op, n, here, NULL);
494+
return true;
495+
}
496+
466497
/*
467498
* Note the addition of a ref on a call.
468499
*/
@@ -510,8 +541,7 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)
510541
spin_unlock_bh(&call->lock);
511542

512543
rxrpc_put_call_slot(call);
513-
514-
del_timer_sync(&call->timer);
544+
rxrpc_delete_call_timer(call);
515545

516546
/* Make sure we don't get any more notifications */
517547
write_lock_bh(&rx->recvmsg_lock);
@@ -618,6 +648,8 @@ static void rxrpc_destroy_call(struct work_struct *work)
618648
struct rxrpc_call *call = container_of(work, struct rxrpc_call, processor);
619649
struct rxrpc_net *rxnet = call->rxnet;
620650

651+
rxrpc_delete_call_timer(call);
652+
621653
rxrpc_put_connection(call->conn);
622654
rxrpc_put_peer(call->peer);
623655
kfree(call->rxtx_buffer);
@@ -652,8 +684,6 @@ void rxrpc_cleanup_call(struct rxrpc_call *call)
652684

653685
memset(&call->sock_node, 0xcd, sizeof(call->sock_node));
654686

655-
del_timer_sync(&call->timer);
656-
657687
ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE);
658688
ASSERT(test_bit(RXRPC_CALL_RELEASED, &call->flags));
659689

0 commit comments

Comments
 (0)