From: Jeffrey E Altman <jaltman@auristor.com>
To: David Howells <dhowells@redhat.com>, netdev@vger.kernel.org
Cc: Hyunwoo Kim <imv4bel@gmail.com>,
Marc Dionne <marc.dionne@auristor.com>,
Jakub Kicinski <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
linux-afs@lists.infradead.org, linux-kernel@vger.kernel.org,
Jiayuan Chen <jiayuan.chen@linux.dev>,
stable@kernel.org
Subject: Re: [PATCH net v2 4/4] rxrpc: Fix RESPONSE packet verification to extract skb to a linear buffer
Date: Wed, 13 May 2026 11:04:41 -0400 [thread overview]
Message-ID: <e34c712e-5b60-45ac-914b-986eb72d31aa@auristor.com> (raw)
In-Reply-To: <20260513131941.1439155-5-dhowells@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 22550 bytes --]
On 5/13/2026 9:19 AM, David Howells wrote:
> This improves the fix for CVE-2026-43500.
>
> Fix the verification of RESPONSE packets to avoid the problem of
> overwriting a RESPONSE packet sent via splice to a local address by
> extracting the contents of the UDP packet into a kmalloc'd linear buffer
> rather than decrypting the data in place in the sk_buff (which may corrupt
> the original buffer).
>
> Further, since the way the RESPONSE data is handled is being overhauled,
> add an XDR decode abstraction that hides the details of buffer advancement
> and length checking. At some point it may be worth seeing if NFS's XDR
> stuff can be used, but that involves linking against the sunrpc module
> which is undesirable.
Thank you for removing the XDR abstraction layer in this revision of the
change.
The commit message still mentions it.
> Fixes: 24481a7f5733 ("rxrpc: Fix conn-level packet handling to unshare RESPONSE packets")
> Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
> Closes: https://lore.kernel.org/r/afKV2zGR6rrelPC7@v4bel/
> 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: Jiayuan Chen <jiayuan.chen@linux.dev>
> cc: linux-afs@lists.infradead.org
> cc: netdev@vger.kernel.org
> cc: stable@kernel.org
> ---
> net/rxrpc/ar-internal.h | 7 +--
> net/rxrpc/conn_event.c | 32 ++++++--------
> net/rxrpc/insecure.c | 5 ++-
> net/rxrpc/rxgk.c | 96 +++++++++++++----------------------------
> net/rxrpc/rxgk_app.c | 44 ++++++++-----------
> net/rxrpc/rxgk_common.h | 92 +--------------------------------------
> net/rxrpc/rxkad.c | 29 +++++--------
> 7 files changed, 81 insertions(+), 224 deletions(-)
>
> diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
> index 783367eea798..98f2165159d7 100644
> --- a/net/rxrpc/ar-internal.h
> +++ b/net/rxrpc/ar-internal.h
> @@ -307,15 +307,16 @@ struct rxrpc_security {
> struct sk_buff *challenge);
>
> /* verify a response */
> - int (*verify_response)(struct rxrpc_connection *,
> - struct sk_buff *);
> + int (*verify_response)(struct rxrpc_connection *conn,
> + struct sk_buff *response_skb,
> + void *response, unsigned int len);
>
> /* clear connection security */
> void (*clear)(struct rxrpc_connection *);
>
> /* Default ticket -> key decoder */
> int (*default_decode_ticket)(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int ticket_offset, unsigned int ticket_len,
> + void *ticket, unsigned int ticket_len,
> struct key **_key);
> };
>
> diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
> index 442414d90ba1..c96ca615b787 100644
> --- a/net/rxrpc/conn_event.c
> +++ b/net/rxrpc/conn_event.c
> @@ -243,28 +243,22 @@ static void rxrpc_call_is_secure(struct rxrpc_call *call)
> static int rxrpc_verify_response(struct rxrpc_connection *conn,
> struct sk_buff *skb)
> {
> + unsigned int len = skb->len - sizeof(struct rxrpc_wire_header);
> + void *buffer;
> int ret;
>
> - if (skb_cloned(skb) || skb_has_frag_list(skb) ||
> - skb_has_shared_frag(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);
> - }
> + buffer = kmalloc(len, GFP_NOFS);
> + if (!buffer)
> + return -ENOMEM;
> +
> + ret = skb_copy_bits(skb, sizeof(struct rxrpc_wire_header), buffer, len);
> + if (ret < 0)
> + goto out;
> +
> + ret = conn->security->verify_response(conn, skb, buffer, len);
>
> +out:
> + kfree(buffer);
> return ret;
> }
>
> diff --git a/net/rxrpc/insecure.c b/net/rxrpc/insecure.c
> index 7a26c6097d03..0b39046bdc61 100644
> --- a/net/rxrpc/insecure.c
> +++ b/net/rxrpc/insecure.c
> @@ -54,9 +54,10 @@ static int none_sendmsg_respond_to_challenge(struct sk_buff *challenge,
> }
>
> static int none_verify_response(struct rxrpc_connection *conn,
> - struct sk_buff *skb)
> + struct sk_buff *response_skb,
> + void *response, unsigned int len)
> {
> - return rxrpc_abort_conn(conn, skb, RX_PROTOCOL_ERROR, -EPROTO,
> + return rxrpc_abort_conn(conn, response_skb, RX_PROTOCOL_ERROR, -EPROTO,
> rxrpc_eproto_rxnull_response);
> }
>
> diff --git a/net/rxrpc/rxgk.c b/net/rxrpc/rxgk.c
> index 85a59d74aea8..6c675590a675 100644
> --- a/net/rxrpc/rxgk.c
> +++ b/net/rxrpc/rxgk.c
> @@ -1080,11 +1080,12 @@ static int rxgk_sendmsg_respond_to_challenge(struct sk_buff *challenge,
> * unsigned int call_numbers<>;
> * };
> */
> -static int rxgk_do_verify_authenticator(struct rxrpc_connection *conn,
> - const struct krb5_enctype *krb5,
> - struct sk_buff *skb,
> - __be32 *p, __be32 *end)
> +static int rxgk_verify_authenticator(struct rxrpc_connection *conn,
> + const struct krb5_enctype *krb5,
> + struct sk_buff *skb,
> + void *auth, unsigned int auth_len)
> {
> + __be32 *p = auth, *end = auth + auth_len;
> u32 app_len, call_count, level, epoch, cid, i;
>
> _enter("");
> @@ -1147,37 +1148,6 @@ static int rxgk_do_verify_authenticator(struct rxrpc_connection *conn,
> return 0;
> }
>
> -/*
> - * Extract the authenticator and verify it.
> - */
> -static int rxgk_verify_authenticator(struct rxrpc_connection *conn,
> - const struct krb5_enctype *krb5,
> - struct sk_buff *skb,
> - unsigned int auth_offset, unsigned int auth_len)
> -{
> - void *auth;
> - __be32 *p;
> - int ret;
> -
> - auth = kmalloc(auth_len, GFP_NOFS);
> - if (!auth)
> - return -ENOMEM;
> -
> - ret = skb_copy_bits(skb, auth_offset, auth, auth_len);
> - if (ret < 0) {
> - ret = rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> - rxgk_abort_resp_short_auth);
> - goto error;
> - }
> -
> - p = auth;
> - ret = rxgk_do_verify_authenticator(conn, krb5, skb, p,
> - p + auth_len / sizeof(*p));
> -error:
> - kfree(auth);
> - return ret;
> -}
> -
> /*
> * Verify a response.
> *
> @@ -1188,49 +1158,45 @@ static int rxgk_verify_authenticator(struct rxrpc_connection *conn,
> * };
> */
> static int rxgk_verify_response(struct rxrpc_connection *conn,
> - struct sk_buff *skb)
> + struct sk_buff *skb,
> + void *buffer, unsigned int len)
> {
> const struct krb5_enctype *krb5;
> struct rxrpc_key_token *token;
> struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
> - struct rxgk_response rhdr;
> + struct rxgk_response *rhdr;
> struct rxgk_context *gk;
> struct key *key = NULL;
> - unsigned int offset = sizeof(struct rxrpc_wire_header);
> - unsigned int len = skb->len - sizeof(struct rxrpc_wire_header);
> - unsigned int token_offset, token_len;
> - unsigned int auth_offset, auth_len;
> + unsigned int resp_token_len, auth_len;
> + void *resp_token, *auth;
> __be32 xauth_len;
> int ret, ec;
>
> _enter("{%d}", conn->debug_id);
>
> /* Parse the RXGK_Response object */
> - if (sizeof(rhdr) + sizeof(__be32) > len)
> - goto short_packet;
> -
> - if (skb_copy_bits(skb, offset, &rhdr, sizeof(rhdr)) < 0)
> + if (len < sizeof(*rhdr) + sizeof(__be32))
> goto short_packet;
> - offset += sizeof(rhdr);
> - len -= sizeof(rhdr);
> -
> - token_offset = offset;
> - token_len = ntohl(rhdr.token_len);
> - if (token_len > len ||
> - xdr_round_up(token_len) + sizeof(__be32) > len)
> + rhdr = buffer;
> + buffer += sizeof(*rhdr);
> + len -= sizeof(*rhdr);
> +
> + resp_token = buffer;
> + resp_token_len = ntohl(rhdr->token_len);
> + if (resp_token_len > len ||
> + xdr_round_up(resp_token_len) + sizeof(__be32) > len)
> goto short_packet;
>
> - trace_rxrpc_rx_response(conn, sp->hdr.serial, 0, sp->hdr.cksum, token_len);
> + trace_rxrpc_rx_response(conn, sp->hdr.serial, 0, sp->hdr.cksum, resp_token_len);
>
> - offset += xdr_round_up(token_len);
> - len -= xdr_round_up(token_len);
> + buffer += xdr_round_up(resp_token_len);
> + len -= xdr_round_up(resp_token_len);
>
> - if (skb_copy_bits(skb, offset, &xauth_len, sizeof(xauth_len)) < 0)
> - goto short_packet;
> - offset += sizeof(xauth_len);
> + xauth_len = *(__be32 *)buffer;
> + buffer += sizeof(xauth_len);
> len -= sizeof(xauth_len);
>
> - auth_offset = offset;
> + auth = buffer;
> auth_len = ntohl(xauth_len);
> if (auth_len > len)
> goto short_packet;
> @@ -1245,7 +1211,7 @@ static int rxgk_verify_response(struct rxrpc_connection *conn,
> * to the app to deal with - which might mean a round trip to
> * userspace.
> */
> - ret = rxgk_extract_token(conn, skb, token_offset, token_len, &key);
> + ret = rxgk_extract_token(conn, skb, resp_token, resp_token_len, &key);
> if (ret < 0)
> goto out;
>
> @@ -1259,7 +1225,7 @@ static int rxgk_verify_response(struct rxrpc_connection *conn,
> */
> token = key->payload.data[0];
> conn->security_level = token->rxgk->level;
> - conn->rxgk.start_time = __be64_to_cpu(rhdr.start_time);
> + conn->rxgk.start_time = __be64_to_cpu(rhdr->start_time);
>
> gk = rxgk_generate_transport_key(conn, token->rxgk, sp->hdr.cksum, GFP_NOFS);
> if (IS_ERR(gk)) {
> @@ -1269,18 +1235,18 @@ static int rxgk_verify_response(struct rxrpc_connection *conn,
>
> krb5 = gk->krb5;
>
> - trace_rxrpc_rx_response(conn, sp->hdr.serial, krb5->etype, sp->hdr.cksum, token_len);
> + trace_rxrpc_rx_response(conn, sp->hdr.serial, krb5->etype, sp->hdr.cksum,
> + resp_token_len);
>
> /* Decrypt, parse and verify the authenticator. */
> - ret = rxgk_decrypt_skb(krb5, gk->resp_enc, skb,
> - &auth_offset, &auth_len, &ec);
> + ret = rxgk_decrypt(krb5, gk->resp_enc, &auth, &auth_len, &ec);
> if (ret < 0) {
> rxrpc_abort_conn(conn, skb, RXGK_SEALEDINCON, ret,
> rxgk_abort_resp_auth_dec);
> goto out_gk;
> }
>
> - ret = rxgk_verify_authenticator(conn, krb5, skb, auth_offset, auth_len);
> + ret = rxgk_verify_authenticator(conn, krb5, skb, auth, auth_len);
> if (ret < 0)
> goto out_gk;
>
> diff --git a/net/rxrpc/rxgk_app.c b/net/rxrpc/rxgk_app.c
> index 0ef2a29eb695..c72e16901973 100644
> --- a/net/rxrpc/rxgk_app.c
> +++ b/net/rxrpc/rxgk_app.c
> @@ -40,7 +40,7 @@
> * };
> */
> int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int ticket_offset, unsigned int ticket_len,
> + void *buffer, unsigned int ticket_len,
> struct key **_key)
> {
> struct rxrpc_key_token *token;
> @@ -49,7 +49,7 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> size_t pre_ticket_len, payload_len;
> unsigned int klen, enctype;
> void *payload, *ticket;
> - __be32 *t, *p, *q, tmp[2];
> + __be32 *t, *p, *q, *tmp;
> int ret;
>
> _enter("");
> @@ -59,10 +59,7 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> rxgk_abort_resp_short_yfs_tkt);
>
> /* Get the session key length */
> - ret = skb_copy_bits(skb, ticket_offset, tmp, sizeof(tmp));
> - if (ret < 0)
> - return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
> - rxgk_abort_resp_short_yfs_klen);
> + tmp = buffer;
> enctype = ntohl(tmp[0]);
> klen = ntohl(tmp[1]);
>
> @@ -84,12 +81,7 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> * it.
> */
> ticket = payload + pre_ticket_len;
> - ret = skb_copy_bits(skb, ticket_offset, ticket, ticket_len);
> - if (ret < 0) {
> - ret = rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
> - rxgk_abort_resp_short_yfs_tkt);
> - goto error;
> - }
> + memcpy(ticket, buffer, ticket_len);
>
> /* Fill out the form header. */
> p = payload;
> @@ -131,7 +123,7 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> goto error;
> }
>
> - /* Ticket read in with skb_copy_bits above */
> + /* Ticket appended above. */
> q += xdr_round_up(ticket_len) / 4;
> if (WARN_ON((unsigned long)q - (unsigned long)payload != payload_len)) {
> ret = -EIO;
> @@ -182,14 +174,15 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> * [tools.ietf.org/html/draft-wilkinson-afs3-rxgk-afs-08 sec 6.1]
> */
> int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int token_offset, unsigned int token_len,
> + void *token, unsigned int token_len,
> struct key **_key)
> {
> const struct krb5_enctype *krb5;
> const struct krb5_buffer *server_secret;
> struct crypto_aead *token_enc = NULL;
> struct key *server_key;
> - unsigned int ticket_offset, ticket_len;
> + unsigned int ticket_len;
> + void *ticket;
> u32 kvno, enctype;
> int ret, ec = 0;
>
> @@ -197,7 +190,7 @@ int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> __be32 kvno;
> __be32 enctype;
> __be32 token_len;
> - } container;
> + } *container;
>
> if (token_len < sizeof(container))
> goto short_packet;
> @@ -206,15 +199,14 @@ int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> * key we should be using. We can then fetch the key, get the secret
> * and set up the crypto to extract the token.
> */
> - if (skb_copy_bits(skb, token_offset, &container, sizeof(container)) < 0)
> - goto short_packet;
> + container = token;
> + token += sizeof(*container);
>
> - kvno = ntohl(container.kvno);
> - enctype = ntohl(container.enctype);
> - ticket_len = ntohl(container.token_len);
> - ticket_offset = token_offset + sizeof(container);
> + kvno = ntohl(container->kvno);
> + enctype = ntohl(container->enctype);
> + ticket_len = ntohl(container->token_len);
>
> - if (ticket_len > xdr_round_down(token_len - sizeof(container)))
> + if (ticket_len > xdr_round_down(token_len - sizeof(*container)))
> goto short_packet;
>
> _debug("KVNO %u", kvno);
> @@ -237,8 +229,8 @@ int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> * gain access to K0, from which we can derive the transport key and
> * thence decode the authenticator.
> */
> - ret = rxgk_decrypt_skb(krb5, token_enc, skb,
> - &ticket_offset, &ticket_len, &ec);
> + ticket = token;
> + ret = rxgk_decrypt(krb5, token_enc, &ticket, &ticket_len, &ec);
> crypto_free_aead(token_enc);
> token_enc = NULL;
> if (ret < 0) {
> @@ -248,7 +240,7 @@ int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> return ret;
> }
>
> - ret = conn->security->default_decode_ticket(conn, skb, ticket_offset,
> + ret = conn->security->default_decode_ticket(conn, skb, ticket,
> ticket_len, _key);
> if (ret < 0)
> goto cant_get_token;
> diff --git a/net/rxrpc/rxgk_common.h b/net/rxrpc/rxgk_common.h
> index 112b5366ce11..3deed5863f5a 100644
> --- a/net/rxrpc/rxgk_common.h
> +++ b/net/rxrpc/rxgk_common.h
> @@ -41,10 +41,10 @@ struct rxgk_context {
> * rxgk_app.c
> */
> int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int ticket_offset, unsigned int ticket_len,
> + void *ticket, unsigned int ticket_len,
> struct key **_key);
> int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int token_offset, unsigned int token_len,
> + void *token, unsigned int token_len,
> struct key **_key);
>
> /*
> @@ -61,50 +61,6 @@ int rxgk_set_up_token_cipher(const struct krb5_buffer *server_key,
> const struct krb5_enctype **_krb5,
> gfp_t gfp);
>
> -/*
> - * Apply decryption and checksumming functions to part of an skbuff. The
> - * offset and length are updated to reflect the actual content of the encrypted
> - * region.
> - */
> -static inline
> -int rxgk_decrypt_skb(const struct krb5_enctype *krb5,
> - struct crypto_aead *aead,
> - struct sk_buff *skb,
> - unsigned int *_offset, unsigned int *_len,
> - int *_error_code)
> -{
> - struct scatterlist sg[16];
> - size_t offset = 0, len = *_len;
> - int nr_sg, ret;
> -
> - sg_init_table(sg, ARRAY_SIZE(sg));
> - nr_sg = skb_to_sgvec(skb, sg, *_offset, len);
> - if (unlikely(nr_sg < 0))
> - return nr_sg;
> -
> - ret = crypto_krb5_decrypt(krb5, aead, sg, nr_sg,
> - &offset, &len);
> - switch (ret) {
> - case 0:
> - *_offset += offset;
> - *_len = len;
> - break;
> - case -EBADMSG: /* Checksum mismatch. */
> - case -EPROTO:
> - *_error_code = RXGK_SEALEDINCON;
> - break;
> - case -EMSGSIZE:
> - *_error_code = RXGK_PACKETSHORT;
> - break;
> - case -ENOPKG: /* Would prefer RXGK_BADETYPE, but not available for YFS. */
> - default:
> - *_error_code = RXGK_INCONSISTENCY;
> - break;
> - }
> -
> - return ret;
> -}
> -
> /*
> * Apply decryption and checksumming functions a flat data buffer. The data
> * point and length are updated to reflect the actual content of the encrypted
> @@ -148,50 +104,6 @@ static inline int rxgk_decrypt(const struct krb5_enctype *krb5,
> return ret;
> }
>
> -/*
> - * Check the MIC on a region of an skbuff. The offset and length are updated
> - * to reflect the actual content of the secure region.
> - */
> -static inline
> -int rxgk_verify_mic_skb(const struct krb5_enctype *krb5,
> - struct crypto_shash *shash,
> - const struct krb5_buffer *metadata,
> - struct sk_buff *skb,
> - unsigned int *_offset, unsigned int *_len,
> - u32 *_error_code)
> -{
> - struct scatterlist sg[16];
> - size_t offset = 0, len = *_len;
> - int nr_sg, ret;
> -
> - sg_init_table(sg, ARRAY_SIZE(sg));
> - nr_sg = skb_to_sgvec(skb, sg, *_offset, len);
> - if (unlikely(nr_sg < 0))
> - return nr_sg;
> -
> - ret = crypto_krb5_verify_mic(krb5, shash, metadata, sg, nr_sg,
> - &offset, &len);
> - switch (ret) {
> - case 0:
> - *_offset += offset;
> - *_len = len;
> - break;
> - case -EBADMSG: /* Checksum mismatch */
> - case -EPROTO:
> - *_error_code = RXGK_SEALEDINCON;
> - break;
> - case -EMSGSIZE:
> - *_error_code = RXGK_PACKETSHORT;
> - break;
> - case -ENOPKG: /* Would prefer RXGK_BADETYPE, but not available for YFS. */
> - default:
> - *_error_code = RXGK_INCONSISTENCY;
> - break;
> - }
> -
> - return ret;
> -}
> -
> /*
> * Check the MIC on a flat buffer. The data pointer and length are updated to
> * reflect the actual content of the secure region.
> diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
> index 075936337836..124ae560234f 100644
> --- a/net/rxrpc/rxkad.c
> +++ b/net/rxrpc/rxkad.c
> @@ -963,7 +963,6 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
> *_expiry = 0;
>
> ASSERT(server_key->payload.data[0] != NULL);
> - ASSERTCMP((unsigned long) ticket & 7UL, ==, 0);
>
> memcpy(&iv, &server_key->payload.data[2], sizeof(iv));
>
> @@ -1112,14 +1111,15 @@ static int rxkad_decrypt_response(struct rxrpc_connection *conn,
> * verify a response
> */
> static int rxkad_verify_response(struct rxrpc_connection *conn,
> - struct sk_buff *skb)
> + struct sk_buff *skb,
> + void *buffer, unsigned int len)
> {
> 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 = NULL;
> + void *ticket;
> u32 version, kvno, ticket_len, level;
> __be32 csum;
> int ret, i;
> @@ -1142,13 +1142,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
> }
> }
>
> - ret = -ENOMEM;
> - response = kzalloc_obj(struct rxkad_response, GFP_NOFS);
> - if (!response)
> - goto error;
> -
> - if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
> - response, sizeof(*response)) < 0) {
> + response = buffer;
> + if (len < sizeof(*response)) {
> ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> rxkad_abort_resp_short);
> goto error;
> @@ -1160,6 +1155,9 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
>
> trace_rxrpc_rx_response(conn, sp->hdr.serial, version, kvno, ticket_len);
>
> + buffer += sizeof(*response);
> + len -= sizeof(*response);
> +
> if (version != RXKAD_VERSION) {
> ret = rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
> rxkad_abort_resp_version);
> @@ -1179,13 +1177,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
> }
>
> /* extract the kerberos ticket and decrypt and decode it */
> - ret = -ENOMEM;
> - ticket = kmalloc(ticket_len, GFP_NOFS);
> - if (!ticket)
> - goto error;
> -
> - if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header) + sizeof(*response),
> - ticket, ticket_len) < 0) {
> + ticket = buffer;
> + if (ticket_len < len) {
> ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> rxkad_abort_resp_short_tkt);
> goto error;
> @@ -1265,8 +1258,6 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
> ret = rxrpc_get_server_data_key(conn, &session_key, expiry, kvno);
>
> error:
> - kfree(ticket);
> - kfree(response);
> key_put(server_key);
> _leave(" = %d", ret);
> return ret;
With the exception of the commit message this change looks good to me.
Reviewed-by: Jeffrey Altman <jaltman@auristor.com>
Thank you.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4467 bytes --]
next prev parent reply other threads:[~2026-05-13 15:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-13 13:19 [PATCH net v2 0/4] rxrpc: Better fix for DATA/RESPONSE decrypt vs splice() David Howells
2026-05-13 13:19 ` [PATCH net v2 1/4] rxrpc: Also unshare DATA/RESPONSE packets when paged frags are present David Howells
2026-05-13 13:19 ` [PATCH net v2 2/4] crypto/krb5, rxrpc: Fix lack of pre-decrypt/pre-verify length checks David Howells
2026-05-13 14:33 ` Jeffrey E Altman
2026-05-13 13:19 ` [PATCH net v2 3/4] rxrpc: Fix DATA decrypt vs splice() by copying data to buffer in recvmsg David Howells
2026-05-13 15:06 ` Jeffrey E Altman
2026-05-13 13:19 ` [PATCH net v2 4/4] rxrpc: Fix RESPONSE packet verification to extract skb to a linear buffer David Howells
2026-05-13 15:04 ` Jeffrey E Altman [this message]
2026-05-13 15:25 ` David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e34c712e-5b60-45ac-914b-986eb72d31aa@auristor.com \
--to=jaltman@auristor.com \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=imv4bel@gmail.com \
--cc=jiayuan.chen@linux.dev \
--cc=kuba@kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.dionne@auristor.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=stable@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox