Commit a05354cb authored by David Howells's avatar David Howells Committed by Greg Kroah-Hartman

rxrpc: Fix read-after-free in rxrpc_queue_local()

commit 06d9532f upstream.

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: 09d2bf59 ("rxrpc: Add a tracepoint to track rxrpc_local refcounting")
Reported-by: syzbot+78e71c5bab4f76a6a719@syzkaller.appspotmail.com
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent f28023c4
...@@ -500,10 +500,10 @@ rxrpc_tx_points; ...@@ -500,10 +500,10 @@ rxrpc_tx_points;
#define E_(a, b) { a, b } #define E_(a, b) { a, b }
TRACE_EVENT(rxrpc_local, TRACE_EVENT(rxrpc_local,
TP_PROTO(struct rxrpc_local *local, enum rxrpc_local_trace op, TP_PROTO(unsigned int local_debug_id, enum rxrpc_local_trace op,
int usage, const void *where), int usage, const void *where),
TP_ARGS(local, op, usage, where), TP_ARGS(local_debug_id, op, usage, where),
TP_STRUCT__entry( TP_STRUCT__entry(
__field(unsigned int, local ) __field(unsigned int, local )
...@@ -513,7 +513,7 @@ TRACE_EVENT(rxrpc_local, ...@@ -513,7 +513,7 @@ TRACE_EVENT(rxrpc_local,
), ),
TP_fast_assign( TP_fast_assign(
__entry->local = local->debug_id; __entry->local = local_debug_id;
__entry->op = op; __entry->op = op;
__entry->usage = usage; __entry->usage = usage;
__entry->where = where; __entry->where = where;
......
...@@ -97,7 +97,7 @@ static struct rxrpc_local *rxrpc_alloc_local(struct rxrpc_net *rxnet, ...@@ -97,7 +97,7 @@ static struct rxrpc_local *rxrpc_alloc_local(struct rxrpc_net *rxnet,
local->debug_id = atomic_inc_return(&rxrpc_debug_id); local->debug_id = atomic_inc_return(&rxrpc_debug_id);
memcpy(&local->srx, srx, sizeof(*srx)); memcpy(&local->srx, srx, sizeof(*srx));
local->srx.srx_service = 0; local->srx.srx_service = 0;
trace_rxrpc_local(local, rxrpc_local_new, 1, NULL); trace_rxrpc_local(local->debug_id, rxrpc_local_new, 1, NULL);
} }
_leave(" = %p", local); _leave(" = %p", local);
...@@ -325,7 +325,7 @@ struct rxrpc_local *rxrpc_get_local(struct rxrpc_local *local) ...@@ -325,7 +325,7 @@ struct rxrpc_local *rxrpc_get_local(struct rxrpc_local *local)
int n; int n;
n = atomic_inc_return(&local->usage); n = atomic_inc_return(&local->usage);
trace_rxrpc_local(local, rxrpc_local_got, n, here); trace_rxrpc_local(local->debug_id, rxrpc_local_got, n, here);
return local; return local;
} }
...@@ -339,7 +339,8 @@ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *local) ...@@ -339,7 +339,8 @@ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *local)
if (local) { if (local) {
int n = atomic_fetch_add_unless(&local->usage, 1, 0); int n = atomic_fetch_add_unless(&local->usage, 1, 0);
if (n > 0) if (n > 0)
trace_rxrpc_local(local, rxrpc_local_got, n + 1, here); trace_rxrpc_local(local->debug_id, rxrpc_local_got,
n + 1, here);
else else
local = NULL; local = NULL;
} }
...@@ -347,16 +348,16 @@ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *local) ...@@ -347,16 +348,16 @@ struct rxrpc_local *rxrpc_get_local_maybe(struct rxrpc_local *local)
} }
/* /*
* Queue a local endpoint unless it has become unreferenced and pass the * Queue a local endpoint and pass the caller's reference to the work item.
* caller's reference to the work item.
*/ */
void rxrpc_queue_local(struct rxrpc_local *local) void rxrpc_queue_local(struct rxrpc_local *local)
{ {
const void *here = __builtin_return_address(0); const void *here = __builtin_return_address(0);
unsigned int debug_id = local->debug_id;
int n = atomic_read(&local->usage);
if (rxrpc_queue_work(&local->processor)) if (rxrpc_queue_work(&local->processor))
trace_rxrpc_local(local, rxrpc_local_queued, trace_rxrpc_local(debug_id, rxrpc_local_queued, n, here);
atomic_read(&local->usage), here);
else else
rxrpc_put_local(local); rxrpc_put_local(local);
} }
...@@ -371,7 +372,7 @@ void rxrpc_put_local(struct rxrpc_local *local) ...@@ -371,7 +372,7 @@ void rxrpc_put_local(struct rxrpc_local *local)
if (local) { if (local) {
n = atomic_dec_return(&local->usage); n = atomic_dec_return(&local->usage);
trace_rxrpc_local(local, rxrpc_local_put, n, here); trace_rxrpc_local(local->debug_id, rxrpc_local_put, n, here);
if (n == 0) if (n == 0)
call_rcu(&local->rcu, rxrpc_local_rcu); call_rcu(&local->rcu, rxrpc_local_rcu);
...@@ -458,7 +459,7 @@ static void rxrpc_local_processor(struct work_struct *work) ...@@ -458,7 +459,7 @@ static void rxrpc_local_processor(struct work_struct *work)
container_of(work, struct rxrpc_local, processor); container_of(work, struct rxrpc_local, processor);
bool again; bool again;
trace_rxrpc_local(local, rxrpc_local_processing, trace_rxrpc_local(local->debug_id, rxrpc_local_processing,
atomic_read(&local->usage), NULL); atomic_read(&local->usage), NULL);
do { do {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment