Commit 0e50d999 authored by David Howells's avatar David Howells Committed by David S. Miller
Browse files

rxrpc: Fix a couple of potential use-after-frees 



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: 84997905 ("rxrpc: Add a tracepoint to follow what recvmsg does")
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent d3805695

include/trace/events/rxrpc.h

+3 −3
Original line number Diff line number Diff line
@@ -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		)
@@ -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;

			   ),

net/rxrpc/recvmsg.c

+8 −6
Original line number Diff line number Diff line
@@ -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;
@@ -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);
@@ -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.
@@ -531,7 +533,7 @@ 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:
@@ -539,14 +541,14 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, 
		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: