| From 7a21d22987eec3d82306816c3e31704169ff51bd Mon Sep 17 00:00:00 2001 |
| From: David Howells <dhowells@redhat.com> |
| Date: Mon, 7 Oct 2019 10:58:29 +0100 |
| Subject: [PATCH] rxrpc: Fix trace-after-put looking at the put peer record |
| |
| commit 55f6c98e3674ce16038a1949c3f9ca5a9a99f289 upstream. |
| |
| rxrpc_put_peer() calls trace_rxrpc_peer() after it has done the decrement |
| of the refcount - which looks at the debug_id in the peer record. But |
| unless the refcount was reduced to zero, we no longer have the right to |
| look in the record and, indeed, it may be deleted by some other thread. |
| |
| Fix this by getting the debug_id out before decrementing the refcount and |
| then passing that into the tracepoint. |
| |
| This can cause the following symptoms: |
| |
| BUG: KASAN: use-after-free in __rxrpc_put_peer net/rxrpc/peer_object.c:411 |
| [inline] |
| BUG: KASAN: use-after-free in rxrpc_put_peer+0x685/0x6a0 |
| net/rxrpc/peer_object.c:435 |
| Read of size 8 at addr ffff888097ec0058 by task syz-executor823/24216 |
| |
| Fixes: 1159d4b496f5 ("rxrpc: Add a tracepoint to track rxrpc_peer refcounting") |
| Reported-by: syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com |
| Signed-off-by: David Howells <dhowells@redhat.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h |
| index 0972c48d81d7..d19f8433d15f 100644 |
| --- a/include/trace/events/rxrpc.h |
| +++ b/include/trace/events/rxrpc.h |
| @@ -525,10 +525,10 @@ TRACE_EVENT(rxrpc_local, |
| ); |
| |
| TRACE_EVENT(rxrpc_peer, |
| - TP_PROTO(struct rxrpc_peer *peer, enum rxrpc_peer_trace op, |
| + TP_PROTO(unsigned int peer_debug_id, enum rxrpc_peer_trace op, |
| int usage, const void *where), |
| |
| - TP_ARGS(peer, op, usage, where), |
| + TP_ARGS(peer_debug_id, op, usage, where), |
| |
| TP_STRUCT__entry( |
| __field(unsigned int, peer ) |
| @@ -538,7 +538,7 @@ TRACE_EVENT(rxrpc_peer, |
| ), |
| |
| TP_fast_assign( |
| - __entry->peer = peer->debug_id; |
| + __entry->peer = peer_debug_id; |
| __entry->op = op; |
| __entry->usage = usage; |
| __entry->where = where; |
| diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c |
| index e6623a978922..64830d8c1fdb 100644 |
| --- a/net/rxrpc/peer_object.c |
| +++ b/net/rxrpc/peer_object.c |
| @@ -381,7 +381,7 @@ struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *peer) |
| int n; |
| |
| n = atomic_inc_return(&peer->usage); |
| - trace_rxrpc_peer(peer, rxrpc_peer_got, n, here); |
| + trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n, here); |
| return peer; |
| } |
| |
| @@ -395,7 +395,7 @@ struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *peer) |
| if (peer) { |
| int n = atomic_fetch_add_unless(&peer->usage, 1, 0); |
| if (n > 0) |
| - trace_rxrpc_peer(peer, rxrpc_peer_got, n + 1, here); |
| + trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n + 1, here); |
| else |
| peer = NULL; |
| } |
| @@ -426,11 +426,13 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer) |
| void rxrpc_put_peer(struct rxrpc_peer *peer) |
| { |
| const void *here = __builtin_return_address(0); |
| + unsigned int debug_id; |
| int n; |
| |
| if (peer) { |
| + debug_id = peer->debug_id; |
| n = atomic_dec_return(&peer->usage); |
| - trace_rxrpc_peer(peer, rxrpc_peer_put, n, here); |
| + trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here); |
| if (n == 0) |
| __rxrpc_put_peer(peer); |
| } |
| @@ -443,10 +445,11 @@ void rxrpc_put_peer(struct rxrpc_peer *peer) |
| void rxrpc_put_peer_locked(struct rxrpc_peer *peer) |
| { |
| const void *here = __builtin_return_address(0); |
| + unsigned int debug_id = peer->debug_id; |
| int n; |
| |
| n = atomic_dec_return(&peer->usage); |
| - trace_rxrpc_peer(peer, rxrpc_peer_put, n, here); |
| + trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here); |
| if (n == 0) { |
| hash_del_rcu(&peer->hash_link); |
| list_del_init(&peer->keepalive_link); |
| -- |
| 2.7.4 |
| |