Skip to content

Commit c4b8f32

Browse files
dhowellsgregkh
dhowells
authored and
gregkh
committed
rxrpc: Fix memory leaks in rxkad_verify_response()
commit 34f61a0 upstream. Fix rxkad_verify_response() to free the ticket and the server key under all circumstances by initialising the ticket pointer to NULL and then making all paths through the function after the first allocation has been done go through a single common epilogue that just releases everything - where all the releases skip on a NULL pointer. Fixes: 57af281 ("rxrpc: Tidy up abort generation infrastructure") Fixes: ec832bd ("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: Simon Horman <horms@kernel.org> cc: linux-afs@lists.infradead.org cc: stable@kernel.org Link: https://patch.msgid.link/20260422161438.2593376-2-dhowells@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 97a9709 commit c4b8f32

1 file changed

Lines changed: 42 additions & 61 deletions

File tree

‎net/rxrpc/rxkad.c‎

Lines changed: 42 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
10471047
struct rxrpc_crypt session_key;
10481048
struct key *server_key;
10491049
time64_t expiry;
1050-
void *ticket;
1050+
void *ticket = NULL;
10511051
u32 version, kvno, ticket_len, level;
10521052
__be32 csum;
10531053
int ret, i;
@@ -1073,13 +1073,13 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
10731073
ret = -ENOMEM;
10741074
response = kzalloc(sizeof(struct rxkad_response), GFP_NOFS);
10751075
if (!response)
1076-
goto temporary_error;
1076+
goto error;
10771077

10781078
if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
10791079
response, sizeof(*response)) < 0) {
1080-
rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
1081-
rxkad_abort_resp_short);
1082-
goto protocol_error;
1080+
ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
1081+
rxkad_abort_resp_short);
1082+
goto error;
10831083
}
10841084

10851085
version = ntohl(response->version);
@@ -1089,133 +1089,114 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
10891089
trace_rxrpc_rx_response(conn, sp->hdr.serial, version, kvno, ticket_len);
10901090

10911091
if (version != RXKAD_VERSION) {
1092-
rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
1093-
rxkad_abort_resp_version);
1094-
goto protocol_error;
1092+
ret = rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
1093+
rxkad_abort_resp_version);
1094+
goto error;
10951095
}
10961096

10971097
if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN) {
1098-
rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
1099-
rxkad_abort_resp_tkt_len);
1100-
goto protocol_error;
1098+
ret = rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
1099+
rxkad_abort_resp_tkt_len);
1100+
goto error;
11011101
}
11021102

11031103
if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5) {
1104-
rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
1105-
rxkad_abort_resp_unknown_tkt);
1106-
goto protocol_error;
1104+
ret = rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
1105+
rxkad_abort_resp_unknown_tkt);
1106+
goto error;
11071107
}
11081108

11091109
/* extract the kerberos ticket and decrypt and decode it */
11101110
ret = -ENOMEM;
11111111
ticket = kmalloc(ticket_len, GFP_NOFS);
11121112
if (!ticket)
1113-
goto temporary_error_free_resp;
1113+
goto error;
11141114

11151115
if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header) + sizeof(*response),
11161116
ticket, ticket_len) < 0) {
1117-
rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
1118-
rxkad_abort_resp_short_tkt);
1119-
goto protocol_error;
1117+
ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
1118+
rxkad_abort_resp_short_tkt);
1119+
goto error;
11201120
}
11211121

11221122
ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
11231123
&session_key, &expiry);
11241124
if (ret < 0)
1125-
goto temporary_error_free_ticket;
1125+
goto error;
11261126

11271127
/* use the session key from inside the ticket to decrypt the
11281128
* response */
11291129
ret = rxkad_decrypt_response(conn, response, &session_key);
11301130
if (ret < 0)
1131-
goto temporary_error_free_ticket;
1131+
goto error;
11321132

11331133
if (ntohl(response->encrypted.epoch) != conn->proto.epoch ||
11341134
ntohl(response->encrypted.cid) != conn->proto.cid ||
11351135
ntohl(response->encrypted.securityIndex) != conn->security_ix) {
1136-
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1137-
rxkad_abort_resp_bad_param);
1138-
goto protocol_error_free;
1136+
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1137+
rxkad_abort_resp_bad_param);
1138+
goto error;
11391139
}
11401140

11411141
csum = response->encrypted.checksum;
11421142
response->encrypted.checksum = 0;
11431143
rxkad_calc_response_checksum(response);
11441144
if (response->encrypted.checksum != csum) {
1145-
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1146-
rxkad_abort_resp_bad_checksum);
1147-
goto protocol_error_free;
1145+
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1146+
rxkad_abort_resp_bad_checksum);
1147+
goto error;
11481148
}
11491149

11501150
for (i = 0; i < RXRPC_MAXCALLS; i++) {
11511151
u32 call_id = ntohl(response->encrypted.call_id[i]);
11521152
u32 counter = READ_ONCE(conn->channels[i].call_counter);
11531153

11541154
if (call_id > INT_MAX) {
1155-
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1156-
rxkad_abort_resp_bad_callid);
1157-
goto protocol_error_free;
1155+
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1156+
rxkad_abort_resp_bad_callid);
1157+
goto error;
11581158
}
11591159

11601160
if (call_id < counter) {
1161-
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1162-
rxkad_abort_resp_call_ctr);
1163-
goto protocol_error_free;
1161+
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1162+
rxkad_abort_resp_call_ctr);
1163+
goto error;
11641164
}
11651165

11661166
if (call_id > counter) {
11671167
if (conn->channels[i].call) {
1168-
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
1168+
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
11691169
rxkad_abort_resp_call_state);
1170-
goto protocol_error_free;
1170+
goto error;
11711171
}
11721172
conn->channels[i].call_counter = call_id;
11731173
}
11741174
}
11751175

11761176
if (ntohl(response->encrypted.inc_nonce) != conn->rxkad.nonce + 1) {
1177-
rxrpc_abort_conn(conn, skb, RXKADOUTOFSEQUENCE, -EPROTO,
1178-
rxkad_abort_resp_ooseq);
1179-
goto protocol_error_free;
1177+
ret = rxrpc_abort_conn(conn, skb, RXKADOUTOFSEQUENCE, -EPROTO,
1178+
rxkad_abort_resp_ooseq);
1179+
goto error;
11801180
}
11811181

11821182
level = ntohl(response->encrypted.level);
11831183
if (level > RXRPC_SECURITY_ENCRYPT) {
1184-
rxrpc_abort_conn(conn, skb, RXKADLEVELFAIL, -EPROTO,
1185-
rxkad_abort_resp_level);
1186-
goto protocol_error_free;
1184+
ret = rxrpc_abort_conn(conn, skb, RXKADLEVELFAIL, -EPROTO,
1185+
rxkad_abort_resp_level);
1186+
goto error;
11871187
}
11881188
conn->security_level = level;
11891189

11901190
/* create a key to hold the security data and expiration time - after
11911191
* this the connection security can be handled in exactly the same way
11921192
* as for a client connection */
11931193
ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno);
1194-
if (ret < 0)
1195-
goto temporary_error_free_ticket;
11961194

1195+
error:
11971196
kfree(ticket);
11981197
kfree(response);
1199-
_leave(" = 0");
1200-
return 0;
1201-
1202-
protocol_error_free:
1203-
kfree(ticket);
1204-
protocol_error:
1205-
kfree(response);
1206-
key_put(server_key);
1207-
return -EPROTO;
1208-
1209-
temporary_error_free_ticket:
1210-
kfree(ticket);
1211-
temporary_error_free_resp:
1212-
kfree(response);
1213-
temporary_error:
1214-
/* Ignore the response packet if we got a temporary error such as
1215-
* ENOMEM. We just want to send the challenge again. Note that we
1216-
* also come out this way if the ticket decryption fails.
1217-
*/
12181198
key_put(server_key);
1199+
_leave(" = %d", ret);
12191200
return ret;
12201201
}
12211202

0 commit comments

Comments
 (0)