Commit 7359db69 authored by David S. Miller's avatar David S. Miller
Browse files


David Howells says:

====================
rxrpc: Syzbot-inspired fixes

Here's a series of patches that fix a number of issues found by syzbot:

 (1) A reference leak on rxrpc_call structs in a sendmsg error path.

 (2) A tracepoint that looked in the rxrpc_peer record after putting it.

     Analogous with this, though not presently detected, the same bug is
     also fixed in relation to rxrpc_connection and rxrpc_call records.

 (3) Peer records don't pin local endpoint records, despite accessing them.

 (4) Access to connection crypto ops to clean up a call after the call's
     ref on that connection has been put.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 57acce31 91fcfbe8
Loading
Loading
Loading
Loading
+9 −9
Original line number Diff line number Diff line
@@ -519,10 +519,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		)
@@ -532,7 +532,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;
@@ -546,10 +546,10 @@ TRACE_EVENT(rxrpc_peer,
	    );

TRACE_EVENT(rxrpc_conn,
	    TP_PROTO(struct rxrpc_connection *conn, enum rxrpc_conn_trace op,
	    TP_PROTO(unsigned int conn_debug_id, enum rxrpc_conn_trace op,
		     int usage, const void *where),

	    TP_ARGS(conn, op, usage, where),
	    TP_ARGS(conn_debug_id, op, usage, where),

	    TP_STRUCT__entry(
		    __field(unsigned int,	conn		)
@@ -559,7 +559,7 @@ TRACE_EVENT(rxrpc_conn,
			     ),

	    TP_fast_assign(
		    __entry->conn = conn->debug_id;
		    __entry->conn = conn_debug_id;
		    __entry->op = op;
		    __entry->usage = usage;
		    __entry->where = where;
@@ -606,10 +606,10 @@ TRACE_EVENT(rxrpc_client,
	    );

TRACE_EVENT(rxrpc_call,
	    TP_PROTO(struct rxrpc_call *call, enum rxrpc_call_trace op,
	    TP_PROTO(unsigned int call_debug_id, enum rxrpc_call_trace op,
		     int usage, const void *where, const void *aux),

	    TP_ARGS(call, op, usage, where, aux),
	    TP_ARGS(call_debug_id, op, usage, where, aux),

	    TP_STRUCT__entry(
		    __field(unsigned int,		call		)
@@ -620,7 +620,7 @@ TRACE_EVENT(rxrpc_call,
			     ),

	    TP_fast_assign(
		    __entry->call = call->debug_id;
		    __entry->call = call_debug_id;
		    __entry->op = op;
		    __entry->usage = usage;
		    __entry->where = where;
+1 −0
Original line number Diff line number Diff line
@@ -556,6 +556,7 @@ struct rxrpc_call {
	struct rxrpc_peer	*peer;		/* Peer record for remote address */
	struct rxrpc_sock __rcu	*socket;	/* socket responsible */
	struct rxrpc_net	*rxnet;		/* Network namespace to which call belongs */
	const struct rxrpc_security *security;	/* applied security module */
	struct mutex		user_mutex;	/* User access mutex */
	unsigned long		ack_at;		/* When deferred ACK needs to happen */
	unsigned long		ack_lost_at;	/* When ACK is figured as lost */
+3 −2
Original line number Diff line number Diff line
@@ -84,7 +84,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
		smp_store_release(&b->conn_backlog_head,
				  (head + 1) & (size - 1));

		trace_rxrpc_conn(conn, rxrpc_conn_new_service,
		trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_service,
				 atomic_read(&conn->usage), here);
	}

@@ -97,7 +97,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
	call->flags |= (1 << RXRPC_CALL_IS_SERVICE);
	call->state = RXRPC_CALL_SERVER_PREALLOC;

	trace_rxrpc_call(call, rxrpc_call_new_service,
	trace_rxrpc_call(call->debug_id, rxrpc_call_new_service,
			 atomic_read(&call->usage),
			 here, (const void *)user_call_ID);

@@ -307,6 +307,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,

	rxrpc_see_call(call);
	call->conn = conn;
	call->security = conn->security;
	call->peer = rxrpc_get_peer(conn->params.peer);
	call->cong_cwnd = call->peer->cong_cwnd;
	return call;
+20 −14
Original line number Diff line number Diff line
@@ -240,7 +240,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
	if (p->intr)
		__set_bit(RXRPC_CALL_IS_INTR, &call->flags);
	call->tx_total_len = p->tx_total_len;
	trace_rxrpc_call(call, rxrpc_call_new_client, atomic_read(&call->usage),
	trace_rxrpc_call(call->debug_id, rxrpc_call_new_client,
			 atomic_read(&call->usage),
			 here, (const void *)p->user_call_ID);

	/* We need to protect a partially set up call against the user as we
@@ -290,8 +291,8 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
	if (ret < 0)
		goto error;

	trace_rxrpc_call(call, rxrpc_call_connected, atomic_read(&call->usage),
			 here, NULL);
	trace_rxrpc_call(call->debug_id, rxrpc_call_connected,
			 atomic_read(&call->usage), here, NULL);

	rxrpc_start_call_timer(call);

@@ -313,8 +314,8 @@ error_dup_user_ID:
error:
	__rxrpc_set_call_completion(call, RXRPC_CALL_LOCAL_ERROR,
				    RX_CALL_DEAD, ret);
	trace_rxrpc_call(call, rxrpc_call_error, atomic_read(&call->usage),
			 here, ERR_PTR(ret));
	trace_rxrpc_call(call->debug_id, rxrpc_call_error,
			 atomic_read(&call->usage), here, ERR_PTR(ret));
	rxrpc_release_call(rx, call);
	mutex_unlock(&call->user_mutex);
	rxrpc_put_call(call, rxrpc_call_put);
@@ -376,7 +377,8 @@ bool rxrpc_queue_call(struct rxrpc_call *call)
	if (n == 0)
		return false;
	if (rxrpc_queue_work(&call->processor))
		trace_rxrpc_call(call, rxrpc_call_queued, n + 1, here, NULL);
		trace_rxrpc_call(call->debug_id, rxrpc_call_queued, n + 1,
				 here, NULL);
	else
		rxrpc_put_call(call, rxrpc_call_put_noqueue);
	return true;
@@ -391,7 +393,8 @@ bool __rxrpc_queue_call(struct rxrpc_call *call)
	int n = atomic_read(&call->usage);
	ASSERTCMP(n, >=, 1);
	if (rxrpc_queue_work(&call->processor))
		trace_rxrpc_call(call, rxrpc_call_queued_ref, n, here, NULL);
		trace_rxrpc_call(call->debug_id, rxrpc_call_queued_ref, n,
				 here, NULL);
	else
		rxrpc_put_call(call, rxrpc_call_put_noqueue);
	return true;
@@ -406,7 +409,8 @@ void rxrpc_see_call(struct rxrpc_call *call)
	if (call) {
		int n = atomic_read(&call->usage);

		trace_rxrpc_call(call, rxrpc_call_seen, n, here, NULL);
		trace_rxrpc_call(call->debug_id, rxrpc_call_seen, n,
				 here, NULL);
	}
}

@@ -418,7 +422,7 @@ void rxrpc_get_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
	const void *here = __builtin_return_address(0);
	int n = atomic_inc_return(&call->usage);

	trace_rxrpc_call(call, op, n, here, NULL);
	trace_rxrpc_call(call->debug_id, op, n, here, NULL);
}

/*
@@ -445,7 +449,8 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)

	_enter("{%d,%d}", call->debug_id, atomic_read(&call->usage));

	trace_rxrpc_call(call, rxrpc_call_release, atomic_read(&call->usage),
	trace_rxrpc_call(call->debug_id, rxrpc_call_release,
			 atomic_read(&call->usage),
			 here, (const void *)call->flags);

	ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE);
@@ -488,10 +493,10 @@ void rxrpc_release_call(struct rxrpc_sock *rx, struct rxrpc_call *call)

	_debug("RELEASE CALL %p (%d CONN %p)", call, call->debug_id, conn);

	if (conn) {
	if (conn)
		rxrpc_disconnect_call(call);
		conn->security->free_call_crypto(call);
	}
	if (call->security)
		call->security->free_call_crypto(call);

	rxrpc_cleanup_ring(call);
	_leave("");
@@ -534,12 +539,13 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
{
	struct rxrpc_net *rxnet = call->rxnet;
	const void *here = __builtin_return_address(0);
	unsigned int debug_id = call->debug_id;
	int n;

	ASSERT(call != NULL);

	n = atomic_dec_return(&call->usage);
	trace_rxrpc_call(call, op, n, here, NULL);
	trace_rxrpc_call(debug_id, op, n, here, NULL);
	ASSERTCMP(n, >=, 0);
	if (n == 0) {
		_debug("call %d dead", call->debug_id);
+7 −2
Original line number Diff line number Diff line
@@ -212,7 +212,8 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
	rxrpc_get_local(conn->params.local);
	key_get(conn->params.key);

	trace_rxrpc_conn(conn, rxrpc_conn_new_client, atomic_read(&conn->usage),
	trace_rxrpc_conn(conn->debug_id, rxrpc_conn_new_client,
			 atomic_read(&conn->usage),
			 __builtin_return_address(0));
	trace_rxrpc_client(conn, -1, rxrpc_client_alloc);
	_leave(" = %p", conn);
@@ -352,6 +353,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,

	if (cp->exclusive) {
		call->conn = candidate;
		call->security = candidate->security;
		call->security_ix = candidate->security_ix;
		call->service_id = candidate->service_id;
		_leave(" = 0 [exclusive %d]", candidate->debug_id);
@@ -403,6 +405,7 @@ static int rxrpc_get_client_conn(struct rxrpc_sock *rx,
candidate_published:
	set_bit(RXRPC_CONN_IN_CLIENT_CONNS, &candidate->flags);
	call->conn = candidate;
	call->security = candidate->security;
	call->security_ix = candidate->security_ix;
	call->service_id = candidate->service_id;
	spin_unlock(&local->client_conns_lock);
@@ -425,6 +428,7 @@ found_extant_conn:

	spin_lock(&conn->channel_lock);
	call->conn = conn;
	call->security = conn->security;
	call->security_ix = conn->security_ix;
	call->service_id = conn->service_id;
	list_add_tail(&call->chan_wait_link, &conn->waiting_calls);
@@ -985,11 +989,12 @@ rxrpc_put_one_client_conn(struct rxrpc_connection *conn)
void rxrpc_put_client_conn(struct rxrpc_connection *conn)
{
	const void *here = __builtin_return_address(0);
	unsigned int debug_id = conn->debug_id;
	int n;

	do {
		n = atomic_dec_return(&conn->usage);
		trace_rxrpc_conn(conn, rxrpc_conn_put_client, n, here);
		trace_rxrpc_conn(debug_id, rxrpc_conn_put_client, n, here);
		if (n > 0)
			return;
		ASSERTCMP(n, >=, 0);
Loading