[PATCH net 1/4] rxrpc: Fix memory leaks in rxkad_verify_response()

[David Howells <dhowells@redhat.com>]

Fix rxkad_verify_response() to free ticket by using a __free() construct
rather than explicitly freeing it.

Also fix rxkad_verify_response() to free the server key by using a __free()
construct.

Fixes: 57af281e5389 ("rxrpc: Tidy up abort generation infrastructure")
Fixes: ec832bd06d6f ("rxrpc: Don't retain the server key in the connection")
Closes: https://sashiko.dev/#/patchset/20260408121252.2249051-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
cc: stable@kernel.org
---
 include/linux/key.h |   2 +
 net/rxrpc/rxkad.c   | 133 +++++++++++++++-----------------------------
 2 files changed, 48 insertions(+), 87 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index 81b8f05c6898..1cafbc3827c2 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -304,6 +304,8 @@ extern void key_put(struct key *key);
 extern bool key_put_tag(struct key_tag *tag);
 extern void key_remove_domain(struct key_tag *domain_tag);
 
+DEFINE_FREE(key_put, struct key *, if (!IS_ERR(_T)) key_put(_T))
+
 static inline struct key *__key_get(struct key *key)
 {
 	refcount_inc(&key->usage);
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index eb7f2769d2b1..0acdc46f42c2 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -1131,21 +1131,20 @@ static int rxkad_decrypt_response(struct rxrpc_connection *conn,
 static int rxkad_verify_response(struct rxrpc_connection *conn,
 				 struct sk_buff *skb)
 {
-	struct rxkad_response *response;
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	struct rxrpc_crypt session_key;
-	struct key *server_key;
 	time64_t expiry;
-	void *ticket;
 	u32 version, kvno, ticket_len, level;
 	__be32 csum;
 	int ret, i;
 
 	_enter("{%d}", conn->debug_id);
 
-	server_key = rxrpc_look_up_server_security(conn, skb, 0, 0);
+	struct key *server_key __free(key_put) =
+		rxrpc_look_up_server_security(conn, skb, 0, 0);
 	if (IS_ERR(server_key)) {
 		ret = PTR_ERR(server_key);
+		server_key = NULL;
 		switch (ret) {
 		case -ENOKEY:
 			return rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, ret,
@@ -1160,16 +1159,15 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 	}
 
 	ret = -ENOMEM;
-	response = kzalloc_obj(struct rxkad_response, GFP_NOFS);
+	struct rxkad_response *response __free(kfree) =
+		kzalloc_obj(struct rxkad_response, GFP_NOFS);
 	if (!response)
 		goto temporary_error;
 
 	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
-			  response, sizeof(*response)) < 0) {
-		rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
-				 rxkad_abort_resp_short);
-		goto protocol_error;
-	}
+			  response, sizeof(*response)) < 0)
+		return rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
+					rxkad_abort_resp_short);
 
 	version = ntohl(response->version);
 	ticket_len = ntohl(response->ticket_len);
@@ -1177,103 +1175,79 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 
 	trace_rxrpc_rx_response(conn, sp->hdr.serial, version, kvno, ticket_len);
 
-	if (version != RXKAD_VERSION) {
-		rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
-				 rxkad_abort_resp_version);
-		goto protocol_error;
-	}
+	if (version != RXKAD_VERSION)
+		return rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
+					rxkad_abort_resp_version);
 
-	if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN) {
-		rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
-				 rxkad_abort_resp_tkt_len);
-		goto protocol_error;
-	}
+	if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN)
+		return rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
+					rxkad_abort_resp_tkt_len);
 
-	if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5) {
-		rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
-				 rxkad_abort_resp_unknown_tkt);
-		goto protocol_error;
-	}
+	if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5)
+		return rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
+					rxkad_abort_resp_unknown_tkt);
 
 	/* extract the kerberos ticket and decrypt and decode it */
 	ret = -ENOMEM;
-	ticket = kmalloc(ticket_len, GFP_NOFS);
+	void *ticket __free(kfree) = kmalloc(ticket_len, GFP_NOFS);
 	if (!ticket)
-		goto temporary_error_free_resp;
+		goto temporary_error;
 
 	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header) + sizeof(*response),
-			  ticket, ticket_len) < 0) {
-		rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
-				 rxkad_abort_resp_short_tkt);
-		goto protocol_error;
-	}
+			  ticket, ticket_len) < 0)
+		return rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
+					rxkad_abort_resp_short_tkt);
 
 	ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
 				   &session_key, &expiry);
 	if (ret < 0)
-		goto temporary_error_free_ticket;
+		goto temporary_error;
 
 	/* use the session key from inside the ticket to decrypt the
 	 * response */
 	ret = rxkad_decrypt_response(conn, response, &session_key);
 	if (ret < 0)
-		goto temporary_error_free_ticket;
+		goto temporary_error;
 
 	if (ntohl(response->encrypted.epoch) != conn->proto.epoch ||
 	    ntohl(response->encrypted.cid) != conn->proto.cid ||
-	    ntohl(response->encrypted.securityIndex) != conn->security_ix) {
-		rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
-				 rxkad_abort_resp_bad_param);
-		goto protocol_error_free;
-	}
+	    ntohl(response->encrypted.securityIndex) != conn->security_ix)
+		return rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
+					rxkad_abort_resp_bad_param);
 
 	csum = response->encrypted.checksum;
 	response->encrypted.checksum = 0;
 	rxkad_calc_response_checksum(response);
-	if (response->encrypted.checksum != csum) {
-		rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
-				 rxkad_abort_resp_bad_checksum);
-		goto protocol_error_free;
-	}
+	if (response->encrypted.checksum != csum)
+		return rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
+					rxkad_abort_resp_bad_checksum);
 
 	for (i = 0; i < RXRPC_MAXCALLS; i++) {
 		u32 call_id = ntohl(response->encrypted.call_id[i]);
 		u32 counter = READ_ONCE(conn->channels[i].call_counter);
 
-		if (call_id > INT_MAX) {
-			rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
-					 rxkad_abort_resp_bad_callid);
-			goto protocol_error_free;
-		}
-
-		if (call_id < counter) {
-			rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
-					 rxkad_abort_resp_call_ctr);
-			goto protocol_error_free;
-		}
-
+		if (call_id > INT_MAX)
+			return rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
+						rxkad_abort_resp_bad_callid);
+		if (call_id < counter)
+			return rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
+						rxkad_abort_resp_call_ctr);
 		if (call_id > counter) {
-			if (conn->channels[i].call) {
-				rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
-						 rxkad_abort_resp_call_state);
-				goto protocol_error_free;
-			}
+			if (conn->channels[i].call)
+				return rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
+							rxkad_abort_resp_call_state);
 			conn->channels[i].call_counter = call_id;
 		}
 	}
 
-	if (ntohl(response->encrypted.inc_nonce) != conn->rxkad.nonce + 1) {
-		rxrpc_abort_conn(conn, skb, RXKADOUTOFSEQUENCE, -EPROTO,
-				 rxkad_abort_resp_ooseq);
-		goto protocol_error_free;
-	}
+	if (ntohl(response->encrypted.inc_nonce) != conn->rxkad.nonce + 1)
+		return rxrpc_abort_conn(conn, skb, RXKADOUTOFSEQUENCE, -EPROTO,
+					rxkad_abort_resp_ooseq);
 
 	level = ntohl(response->encrypted.level);
-	if (level > RXRPC_SECURITY_ENCRYPT) {
-		rxrpc_abort_conn(conn, skb, RXKADLEVELFAIL, -EPROTO,
-				 rxkad_abort_resp_level);
-		goto protocol_error_free;
-	}
+	if (level > RXRPC_SECURITY_ENCRYPT)
+		return rxrpc_abort_conn(conn, skb, RXKADLEVELFAIL, -EPROTO,
+					rxkad_abort_resp_level);
 	conn->security_level = level;
 
 	/* create a key to hold the security data and expiration time - after
@@ -1281,30 +1255,15 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 	 * as for a client connection */
 	ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno);
 	if (ret < 0)
-		goto temporary_error_free_ticket;
-
-	kfree(ticket);
-	kfree(response);
+		goto temporary_error;
 	_leave(" = 0");
 	return 0;
 
-protocol_error_free:
-	kfree(ticket);
-protocol_error:
-	kfree(response);
-	key_put(server_key);
-	return -EPROTO;
-
-temporary_error_free_ticket:
-	kfree(ticket);
-temporary_error_free_resp:
-	kfree(response);
 temporary_error:
 	/* Ignore the response packet if we got a temporary error such as
 	 * ENOMEM.  We just want to send the challenge again.  Note that we
 	 * also come out this way if the ticket decryption fails.
 	 */
-	key_put(server_key);
 	return ret;
 }