Skip to content

Commit

Permalink
rxrpc: Fix a couple of potential use-after-frees
Browse files Browse the repository at this point in the history
At the end of rxrpc_recvmsg(), if a call is found, the call is put and then
a trace line is emitted referencing that call in a couple of places - but
the call may have been deallocated by the time those traces happen.

Fix this by stashing the call debug_id in a variable and passing that to
the tracepoint rather than the call pointer.

Fixes: 8499790 ("rxrpc: Add a tracepoint to follow what recvmsg does")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David Howells authored and David S. Miller committed Dec 28, 2022
1 parent d380569 commit 0e50d99
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
6 changes: 3 additions & 3 deletions include/trace/events/rxrpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -1062,10 +1062,10 @@ TRACE_EVENT(rxrpc_receive,
);

TRACE_EVENT(rxrpc_recvmsg,
TP_PROTO(struct rxrpc_call *call, enum rxrpc_recvmsg_trace why,
TP_PROTO(unsigned int call_debug_id, enum rxrpc_recvmsg_trace why,
int ret),

TP_ARGS(call, why, ret),
TP_ARGS(call_debug_id, why, ret),

TP_STRUCT__entry(
__field(unsigned int, call )
Expand All @@ -1074,7 +1074,7 @@ TRACE_EVENT(rxrpc_recvmsg,
),

TP_fast_assign(
__entry->call = call ? call->debug_id : 0;
__entry->call = call_debug_id;
__entry->why = why;
__entry->ret = ret;
),
Expand Down
14 changes: 8 additions & 6 deletions net/rxrpc/recvmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,14 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
struct rxrpc_call *call;
struct rxrpc_sock *rx = rxrpc_sk(sock->sk);
struct list_head *l;
unsigned int call_debug_id = 0;
size_t copied = 0;
long timeo;
int ret;

DEFINE_WAIT(wait);

trace_rxrpc_recvmsg(NULL, rxrpc_recvmsg_enter, 0);
trace_rxrpc_recvmsg(0, rxrpc_recvmsg_enter, 0);

if (flags & (MSG_OOB | MSG_TRUNC))
return -EOPNOTSUPP;
Expand Down Expand Up @@ -431,7 +432,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
if (list_empty(&rx->recvmsg_q)) {
if (signal_pending(current))
goto wait_interrupted;
trace_rxrpc_recvmsg(NULL, rxrpc_recvmsg_wait, 0);
trace_rxrpc_recvmsg(0, rxrpc_recvmsg_wait, 0);
timeo = schedule_timeout(timeo);
}
finish_wait(sk_sleep(&rx->sk), &wait);
Expand All @@ -450,7 +451,8 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
rxrpc_get_call(call, rxrpc_call_get_recvmsg);
write_unlock(&rx->recvmsg_lock);

trace_rxrpc_recvmsg(call, rxrpc_recvmsg_dequeue, 0);
call_debug_id = call->debug_id;
trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_dequeue, 0);

/* We're going to drop the socket lock, so we need to lock the call
* against interference by sendmsg.
Expand Down Expand Up @@ -531,22 +533,22 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
error_unlock_call:
mutex_unlock(&call->user_mutex);
rxrpc_put_call(call, rxrpc_call_put_recvmsg);
trace_rxrpc_recvmsg(call, rxrpc_recvmsg_return, ret);
trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_return, ret);
return ret;

error_requeue_call:
if (!(flags & MSG_PEEK)) {
write_lock(&rx->recvmsg_lock);
list_add(&call->recvmsg_link, &rx->recvmsg_q);
write_unlock(&rx->recvmsg_lock);
trace_rxrpc_recvmsg(call, rxrpc_recvmsg_requeue, 0);
trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_requeue, 0);
} else {
rxrpc_put_call(call, rxrpc_call_put_recvmsg);
}
error_no_call:
release_sock(&rx->sk);
error_trace:
trace_rxrpc_recvmsg(call, rxrpc_recvmsg_return, ret);
trace_rxrpc_recvmsg(call_debug_id, rxrpc_recvmsg_return, ret);
return ret;

wait_interrupted:
Expand Down

0 comments on commit 0e50d99

Please sign in to comment.