Skip to content

Commit 06d9532

Browse files
committed
rxrpc: Fix read-after-free in rxrpc_queue_local()
rxrpc_queue_local() attempts to queue the local endpoint it is given and then, if successful, prints a trace line. The trace line includes the current usage count - but we're not allowed to look at the local endpoint at this point as we passed our ref on it to the workqueue. Fix this by reading the usage count before queuing the work item. Also fix the reading of local->debug_id for trace lines, which must be done with the same consideration as reading the usage count. Fixes: 09d2bf5 ("rxrpc: Add a tracepoint to track rxrpc_local refcounting") Reported-by: syzbot+78e71c5bab4f76a6a719@syzkaller.appspotmail.com Signed-off-by: David Howells <dhowells@redhat.com>
1 parent b00df84 commit 06d9532

File tree

2 files changed

+13
-12
lines changed

2 files changed

+13
-12
lines changed

include/trace/events/rxrpc.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -498,10 +498,10 @@ rxrpc_tx_points;
498498
#define E_(a, b) { a, b }
499499

500500
TRACE_EVENT(rxrpc_local,
501-
TP_PROTO(struct rxrpc_local *local, enum rxrpc_local_trace op,
501+
TP_PROTO(unsigned int local_debug_id, enum rxrpc_local_trace op,
502502
int usage, const void *where),
503503

504-
TP_ARGS(local, op, usage, where),
504+
TP_ARGS(local_debug_id, op, usage, where),
505505

506506
TP_STRUCT__entry(
507507
__field(unsigned int, local )
@@ -511,7 +511,7 @@ TRACE_EVENT(rxrpc_local,
511511
),
512512

513513
TP_fast_assign(
514-
__entry->local = local->debug_id;
514+
__entry->local = local_debug_id;
515515
__entry->op = op;
516516
__entry->usage = usage;
517517
__entry->where = where;

net/rxrpc/local_object.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ static struct rxrpc_local *rxrpc_alloc_local(struct rxrpc_net *rxnet,
9393
local->debug_id = atomic_inc_return(&rxrpc_debug_id);
9494
memcpy(&local->srx, srx, sizeof(*srx));
9595
local->srx.srx_service = 0;
96-
trace_rxrpc_local(local, rxrpc_local_new, 1, NULL);
96+
trace_rxrpc_local(local->debug_id, rxrpc_local_new, 1, NULL);
9797
}
9898

9999
_leave(" = %p", local);
@@ -321,7 +321,7 @@ struct rxrpc_local *rxrpc_get_local(struct rxrpc_local *local)
321321
int n;
322322

323323
n = atomic_inc_return(&local->usage);
324-
trace_rxrpc_local(local, rxrpc_local_got, n, here);
324+
trace_rxrpc_local(local->debug_id, rxrpc_local_got, n, here);
325325
return local;
326326
}
327327

@@ -335,24 +335,25 @@ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *local)
335335
if (local) {
336336
int n = atomic_fetch_add_unless(&local->usage, 1, 0);
337337
if (n > 0)
338-
trace_rxrpc_local(local, rxrpc_local_got, n + 1, here);
338+
trace_rxrpc_local(local->debug_id, rxrpc_local_got,
339+
n + 1, here);
339340
else
340341
local = NULL;
341342
}
342343
return local;
343344
}
344345

345346
/*
346-
* Queue a local endpoint unless it has become unreferenced and pass the
347-
* caller's reference to the work item.
347+
* Queue a local endpoint and pass the caller's reference to the work item.
348348
*/
349349
void rxrpc_queue_local(struct rxrpc_local *local)
350350
{
351351
const void *here = __builtin_return_address(0);
352+
unsigned int debug_id = local->debug_id;
353+
int n = atomic_read(&local->usage);
352354

353355
if (rxrpc_queue_work(&local->processor))
354-
trace_rxrpc_local(local, rxrpc_local_queued,
355-
atomic_read(&local->usage), here);
356+
trace_rxrpc_local(debug_id, rxrpc_local_queued, n, here);
356357
else
357358
rxrpc_put_local(local);
358359
}
@@ -367,7 +368,7 @@ void rxrpc_put_local(struct rxrpc_local *local)
367368

368369
if (local) {
369370
n = atomic_dec_return(&local->usage);
370-
trace_rxrpc_local(local, rxrpc_local_put, n, here);
371+
trace_rxrpc_local(local->debug_id, rxrpc_local_put, n, here);
371372

372373
if (n == 0)
373374
call_rcu(&local->rcu, rxrpc_local_rcu);
@@ -456,7 +457,7 @@ static void rxrpc_local_processor(struct work_struct *work)
456457
container_of(work, struct rxrpc_local, processor);
457458
bool again;
458459

459-
trace_rxrpc_local(local, rxrpc_local_processing,
460+
trace_rxrpc_local(local->debug_id, rxrpc_local_processing,
460461
atomic_read(&local->usage), NULL);
461462

462463
do {

0 commit comments

Comments
 (0)