[PATCH net 1/4] rxrpc: Fix memory leaks in rxkad_verify_response()
David Howells
dhowells at redhat.com
Mon Apr 20 07:58:54 PDT 2026
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 at redhat.com>
cc: Marc Dionne <marc.dionne at auristor.com>
cc: Jeffrey Altman <jaltman at auristor.com>
cc: Eric Dumazet <edumazet at google.com>
cc: "David S. Miller" <davem at davemloft.net>
cc: Jakub Kicinski <kuba at kernel.org>
cc: Paolo Abeni <pabeni at redhat.com>
cc: Simon Horman <horms at kernel.org>
cc: linux-afs at lists.infradead.org
cc: netdev at vger.kernel.org
cc: stable at 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;
}
More information about the linux-afs
mailing list