Merge branch 'rxrpc-miscellaneous-fixes'

David Howells says:

====================
rxrpc: Miscellaneous fixes

Here are some fixes for rxrpc, as found by Sashiko[1]:

 (1) Fix leaks in rxkad_verify_response().

 (2) Fix handling of rxkad-encrypted packets with crypto-misaligned
     lengths.

 (3) Fix problem with unsharing DATA packets potentially causing a crash in
     the caller.

 (4) Fix lack of unsharing of RESPONSE packets.

 (5) Fix integer overflow in RxGK ticket length check.

 (6) Fix missing length check in RxKAD tickets.
====================

Link: https://patch.msgid.link/20260422161438.2593376-1-dhowells@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-04-23 12:40:54 -07:00
commit 27ae4bcf4d
10 changed files with 106 additions and 100 deletions

View File

@ -37,6 +37,7 @@
EM(rxkad_abort_1_short_encdata, "rxkad1-short-encdata") \
EM(rxkad_abort_1_short_header, "rxkad1-short-hdr") \
EM(rxkad_abort_2_short_check, "rxkad2-short-check") \
EM(rxkad_abort_2_crypto_unaligned, "rxkad2-crypto-unaligned") \
EM(rxkad_abort_2_short_data, "rxkad2-short-data") \
EM(rxkad_abort_2_short_header, "rxkad2-short-hdr") \
EM(rxkad_abort_2_short_len, "rxkad2-short-len") \
@ -161,8 +162,6 @@
E_(rxrpc_call_poke_timer_now, "Timer-now")
#define rxrpc_skb_traces \
EM(rxrpc_skb_eaten_by_unshare, "ETN unshare ") \
EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \
EM(rxrpc_skb_get_call_rx, "GET call-rx ") \
EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \
EM(rxrpc_skb_get_conn_work, "GET conn-work") \
@ -189,6 +188,7 @@
EM(rxrpc_skb_put_purge, "PUT purge ") \
EM(rxrpc_skb_put_purge_oob, "PUT purge-oob") \
EM(rxrpc_skb_put_response, "PUT response ") \
EM(rxrpc_skb_put_response_copy, "PUT resp-cpy ") \
EM(rxrpc_skb_put_rotate, "PUT rotate ") \
EM(rxrpc_skb_put_unknown, "PUT unknown ") \
EM(rxrpc_skb_see_conn_work, "SEE conn-work") \
@ -197,6 +197,7 @@
EM(rxrpc_skb_see_recvmsg_oob, "SEE recvm-oob") \
EM(rxrpc_skb_see_reject, "SEE reject ") \
EM(rxrpc_skb_see_rotate, "SEE rotate ") \
EM(rxrpc_skb_see_unshare_nomem, "SEE unshar-nm") \
E_(rxrpc_skb_see_version, "SEE version ")
#define rxrpc_local_traces \

View File

@ -1486,7 +1486,6 @@ int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
void rxrpc_kernel_data_consumed(struct rxrpc_call *, struct sk_buff *);
void rxrpc_new_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_see_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_eaten_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_get_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_free_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_purge_queue(struct sk_buff_head *);

View File

@ -332,7 +332,24 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
saw_ack |= sp->hdr.type == RXRPC_PACKET_TYPE_ACK;
rxrpc_input_call_packet(call, skb);
if (sp->hdr.securityIndex != 0 &&
skb_cloned(skb)) {
/* Unshare the packet so that it can be
* modified by in-place decryption.
*/
struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
if (nskb) {
rxrpc_new_skb(nskb, rxrpc_skb_new_unshared);
rxrpc_input_call_packet(call, nskb);
rxrpc_free_skb(nskb, rxrpc_skb_put_call_rx);
} else {
/* OOM - Drop the packet. */
rxrpc_see_skb(skb, rxrpc_skb_see_unshare_nomem);
}
} else {
rxrpc_input_call_packet(call, skb);
}
rxrpc_free_skb(skb, rxrpc_skb_put_call_rx);
did_receive = true;
}

View File

@ -240,6 +240,33 @@ static void rxrpc_call_is_secure(struct rxrpc_call *call)
rxrpc_notify_socket(call);
}
static int rxrpc_verify_response(struct rxrpc_connection *conn,
struct sk_buff *skb)
{
int ret;
if (skb_cloned(skb)) {
/* Copy the packet if shared so that we can do in-place
* decryption.
*/
struct sk_buff *nskb = skb_copy(skb, GFP_NOFS);
if (nskb) {
rxrpc_new_skb(nskb, rxrpc_skb_new_unshared);
ret = conn->security->verify_response(conn, nskb);
rxrpc_free_skb(nskb, rxrpc_skb_put_response_copy);
} else {
/* OOM - Drop the packet. */
rxrpc_see_skb(skb, rxrpc_skb_see_unshare_nomem);
ret = -ENOMEM;
}
} else {
ret = conn->security->verify_response(conn, skb);
}
return ret;
}
/*
* connection-level Rx packet processor
*/
@ -270,7 +297,7 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
}
spin_unlock_irq(&conn->state_lock);
ret = conn->security->verify_response(conn, skb);
ret = rxrpc_verify_response(conn, skb);
if (ret < 0)
return ret;

View File

@ -192,13 +192,12 @@ static bool rxrpc_extract_abort(struct sk_buff *skb)
/*
* Process packets received on the local endpoint
*/
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff *skb)
{
struct rxrpc_connection *conn;
struct sockaddr_rxrpc peer_srx;
struct rxrpc_skb_priv *sp;
struct rxrpc_peer *peer = NULL;
struct sk_buff *skb = *_skb;
bool ret = false;
skb_pull(skb, sizeof(struct udphdr));
@ -244,25 +243,6 @@ static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_call);
if (sp->hdr.seq == 0)
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_seq);
/* Unshare the packet so that it can be modified for in-place
* decryption.
*/
if (sp->hdr.securityIndex != 0) {
skb = skb_unshare(skb, GFP_ATOMIC);
if (!skb) {
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare_nomem);
*_skb = NULL;
return just_discard;
}
if (skb != *_skb) {
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare);
*_skb = skb;
rxrpc_new_skb(skb, rxrpc_skb_new_unshared);
sp = rxrpc_skb(skb);
}
}
break;
case RXRPC_PACKET_TYPE_CHALLENGE:
@ -494,7 +474,7 @@ int rxrpc_io_thread(void *data)
switch (skb->mark) {
case RXRPC_SKB_MARK_PACKET:
skb->priority = 0;
if (!rxrpc_input_packet(local, &skb))
if (!rxrpc_input_packet(local, skb))
rxrpc_reject_packet(local, skb);
trace_rxrpc_rx_done(skb->mark, skb->priority);
rxrpc_free_skb(skb, rxrpc_skb_put_input);

View File

@ -502,6 +502,10 @@ static int rxrpc_preparse(struct key_preparsed_payload *prep)
if (v1->security_index != RXRPC_SECURITY_RXKAD)
goto error;
ret = -EKEYREJECTED;
if (v1->ticket_length > AFSTOKEN_RK_TIX_MAX)
goto error;
plen = sizeof(*token->kad) + v1->ticket_length;
prep->quotalen += plen + sizeof(*token);

View File

@ -214,7 +214,7 @@ int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
ticket_len = ntohl(container.token_len);
ticket_offset = token_offset + sizeof(container);
if (xdr_round_up(ticket_len) > token_len - sizeof(container))
if (ticket_len > xdr_round_down(token_len - sizeof(container)))
goto short_packet;
_debug("KVNO %u", kvno);

View File

@ -34,6 +34,7 @@ struct rxgk_context {
};
#define xdr_round_up(x) (round_up((x), sizeof(__be32)))
#define xdr_round_down(x) (round_down((x), sizeof(__be32)))
#define xdr_object_len(x) (4 + xdr_round_up(x))
/*

View File

@ -510,6 +510,9 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb,
return rxrpc_abort_eproto(call, skb, RXKADSEALEDINCON,
rxkad_abort_2_short_header);
/* Don't let the crypto algo see a misaligned length. */
sp->len = round_down(sp->len, 8);
/* Decrypt the skbuff in-place. TODO: We really want to decrypt
* directly into the target buffer.
*/
@ -543,8 +546,10 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb,
if (sg != _sg)
kfree(sg);
if (ret < 0) {
WARN_ON_ONCE(ret != -ENOMEM);
return ret;
if (ret == -ENOMEM)
return ret;
return rxrpc_abort_eproto(call, skb, RXKADSEALEDINCON,
rxkad_abort_2_crypto_unaligned);
}
/* Extract the decrypted packet length */
@ -1136,7 +1141,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
struct rxrpc_crypt session_key;
struct key *server_key;
time64_t expiry;
void *ticket;
void *ticket = NULL;
u32 version, kvno, ticket_len, level;
__be32 csum;
int ret, i;
@ -1162,13 +1167,13 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
ret = -ENOMEM;
response = kzalloc_obj(struct rxkad_response, GFP_NOFS);
if (!response)
goto temporary_error;
goto 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;
ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
rxkad_abort_resp_short);
goto error;
}
version = ntohl(response->version);
@ -1178,62 +1183,62 @@ 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;
ret = rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
rxkad_abort_resp_version);
goto error;
}
if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN) {
rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
rxkad_abort_resp_tkt_len);
goto protocol_error;
ret = rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
rxkad_abort_resp_tkt_len);
goto error;
}
if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5) {
rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
rxkad_abort_resp_unknown_tkt);
goto protocol_error;
ret = rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
rxkad_abort_resp_unknown_tkt);
goto error;
}
/* extract the kerberos ticket and decrypt and decode it */
ret = -ENOMEM;
ticket = kmalloc(ticket_len, GFP_NOFS);
if (!ticket)
goto temporary_error_free_resp;
goto 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;
ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
rxkad_abort_resp_short_tkt);
goto error;
}
ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
&session_key, &expiry);
if (ret < 0)
goto temporary_error_free_ticket;
goto 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 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;
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_bad_param);
goto error;
}
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;
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_bad_checksum);
goto error;
}
for (i = 0; i < RXRPC_MAXCALLS; i++) {
@ -1241,38 +1246,38 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
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;
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_bad_callid);
goto error;
}
if (call_id < counter) {
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_call_ctr);
goto protocol_error_free;
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_call_ctr);
goto error;
}
if (call_id > counter) {
if (conn->channels[i].call) {
rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
ret = rxrpc_abort_conn(conn, skb, RXKADSEALEDINCON, -EPROTO,
rxkad_abort_resp_call_state);
goto protocol_error_free;
goto error;
}
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;
ret = rxrpc_abort_conn(conn, skb, RXKADOUTOFSEQUENCE, -EPROTO,
rxkad_abort_resp_ooseq);
goto error;
}
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;
ret = rxrpc_abort_conn(conn, skb, RXKADLEVELFAIL, -EPROTO,
rxkad_abort_resp_level);
goto error;
}
conn->security_level = level;
@ -1280,31 +1285,12 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
* this the connection security can be handled in exactly the same way
* as for a client connection */
ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno);
if (ret < 0)
goto temporary_error_free_ticket;
error:
kfree(ticket);
kfree(response);
_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);
_leave(" = %d", ret);
return ret;
}

View File

@ -46,15 +46,6 @@ void rxrpc_get_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
skb_get(skb);
}
/*
* Note the dropping of a ref on a socket buffer by the core.
*/
void rxrpc_eaten_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
{
int n = atomic_inc_return(&rxrpc_n_rx_skbs);
trace_rxrpc_skb(skb, 0, n, why);
}
/*
* Note the destruction of a socket buffer.
*/