[PATCH net 3/3] rxrpc: Fix RESPONSE packet verification to extract skb to a linear buffer
Jeffrey Altman
jaltman at auristor.com
Tue May 12 01:22:06 PDT 2026
> On May 11, 2026, at 12:07 PM, David Howells <dhowells at redhat.com> 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.
>
Might be worth adding a back-porting note that the rxgk changes can be
safely dropped if the kernel does not support rxgk.
> Fixes: 24481a7f5733 ("rxrpc: Fix conn-level packet handling to unshare RESPONSE packets")
> Reported-by: Hyunwoo Kim <imv4bel at gmail.com>
> Closes: https://lore.kernel.org/r/afKV2zGR6rrelPC7@v4bel/
> 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: Jiayuan Chen <jiayuan.chen at linux.dev>
> cc: linux-afs at lists.infradead.org
> cc: netdev at vger.kernel.org
> cc: stable at kernel.org
> ---
> net/rxrpc/ar-internal.h | 70 +++++++++++++++++--
> net/rxrpc/conn_event.c | 35 +++++-----
> net/rxrpc/insecure.c | 5 +-
> net/rxrpc/protocol.h | 1 -
> net/rxrpc/rxgk.c | 146 +++++++++++++---------------------------
> net/rxrpc/rxgk_app.c | 91 +++++++++++--------------
> net/rxrpc/rxgk_common.h | 115 ++++---------------------------
> net/rxrpc/rxkad.c | 89 ++++++++++--------------
> 8 files changed, 219 insertions(+), 333 deletions(-)
>
> diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
> index 783367eea798..610fa208157b 100644
> --- a/net/rxrpc/ar-internal.h
> +++ b/net/rxrpc/ar-internal.h
> @@ -33,6 +33,13 @@ struct rxrpc_txbuf;
> struct rxrpc_txqueue;
> struct rxgk_context;
>
> +typedef __be32 xdr_t;
> +
> +struct xdr_buffer {
> + xdr_t *p; /* Current decode point */
> + unsigned int len; /* Amount left in buffer */
> +};
> +
> /*
> * Mark applied to socket buffers in skb->mark. skb->priority is used
> * to pass supplementary information.
> @@ -307,16 +314,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,
> + struct xdr_buffer *response);
>
> /* 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,
> - struct key **_key);
> + struct xdr_buffer *ticket, struct key **_key);
> };
>
> /*
> @@ -1591,6 +1598,61 @@ static inline unsigned int rxrpc_tx_in_flight(const struct rxrpc_call *call)
> return call->tx_nr_sent - rxrpc_left_out(call) + call->tx_nr_resent;
> }
>
> +#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))
> +
> +/*
> + * Check the amount of data remaining in an XDR buffer.
> + */
> +static inline bool xdr_check(struct xdr_buffer *buf, unsigned int bytes)
> +{
> + if (bytes > buf->len ||
> + xdr_round_up(bytes) > buf->len)
> + return false;
> + return true;
> +}
> +
> +/*
> + * Grab a region in an XDR buffer and advance the buffer position, returning a
> + * pointer to the start of the region or NULL if there's insufficient data.
> + */
> +static inline void *xdr_extract_region(struct xdr_buffer *buf, unsigned int bytes)
> +{
> + xdr_t *p = buf->p;
> +
> + if (!xdr_check(buf, bytes))
> + return NULL;
> +
> + bytes = xdr_round_up(bytes);
> + buf->p += bytes / sizeof(*buf->p);
> + buf->len -= bytes;
> + return p;
> +}
> +
> +/*
> + * Decode an XDR word.
> + */
> +static inline u32 xdr_dec(xdr_t x)
> +{
> + return ntohl(x);
> +}
> +
> +/*
> + * Grab a blob with size from an XDR buffer and advance the buffer position.
> + */
> +static inline bool xdr_extract_blob(struct xdr_buffer *buf, struct xdr_buffer *blob)
> +{
> + xdr_t *xsize;
> +
> + xsize = xdr_extract_region(buf, sizeof(*xsize));
> + if (!xsize)
> + return false;
> + blob->len = xdr_dec(*xsize);
> + blob->p = xdr_extract_region(buf, blob->len);
> + return blob->p != NULL;
> +}
> +
> /*
> * debug tracing
> */
> diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
> index 442414d90ba1..26cab3d2075e 100644
> --- a/net/rxrpc/conn_event.c
> +++ b/net/rxrpc/conn_event.c
> @@ -243,28 +243,25 @@ static void rxrpc_call_is_secure(struct rxrpc_call *call)
> static int rxrpc_verify_response(struct rxrpc_connection *conn,
> struct sk_buff *skb)
> {
> + struct xdr_buffer response = {
> + .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(response.len, GFP_NOFS);
> + if (!buffer)
> + return ret;
> +
> + ret = skb_copy_bits(skb, sizeof(struct rxrpc_wire_header), buffer, response.len);
> + if (ret < 0)
> + goto out;
> +
> + response.p = buffer;
> + ret = conn->security->verify_response(conn, skb, &response);
>
> +out:
> + kfree(buffer);
> return ret;
> }
>
> diff --git a/net/rxrpc/insecure.c b/net/rxrpc/insecure.c
> index 7a26c6097d03..dd1827f683bc 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,
> + struct xdr_buffer *response)
> {
> - 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/protocol.h b/net/rxrpc/protocol.h
> index f8bfec12bc7e..6e02a84fd370 100644
> --- a/net/rxrpc/protocol.h
> +++ b/net/rxrpc/protocol.h
> @@ -198,7 +198,6 @@ struct rxgk_header {
> */
> struct rxgk_response {
> __be64 start_time;
> - __be32 token_len;
> } __packed;
>
> #endif /* _LINUX_RXRPC_PACKET_H */
> diff --git a/net/rxrpc/rxgk.c b/net/rxrpc/rxgk.c
> index 88e651dd0e90..fd73b1ff3b97 100644
> --- a/net/rxrpc/rxgk.c
> +++ b/net/rxrpc/rxgk.c
> @@ -524,43 +524,42 @@ static int rxgk_verify_packet_encrypted(struct rxrpc_call *call,
> {
> struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
> struct rxgk_header *hdr;
> - unsigned int offset = 0, len = call->rx_dec_len;
> - void *data = call->rx_dec_buffer;
> + struct xdr_buffer xdr = {
> + .p = call->rx_dec_buffer,
> + .len = call->rx_dec_len,
> + };
> int ret;
> u32 ac = 0;
>
> _enter("");
>
> - ret = rxgk_decrypt(gk->krb5, gk->rx_enc, data, &offset, &len, &ac);
> + ret = rxgk_decrypt(gk->krb5, gk->rx_enc, &xdr, &ac);
> if (ret < 0) {
> if (ret != -ENOMEM)
> rxrpc_abort_eproto(call, skb, ac, rxgk_abort_2_decrypt_eproto);
> goto error;
> }
>
> - if (len < sizeof(*hdr)) {
> + /* Extract the header from the skb */
> + hdr = xdr_extract_region(&xdr, sizeof(*hdr));
> + if (!hdr) {
> ret = rxrpc_abort_eproto(call, skb, RXGK_PACKETSHORT,
> rxgk_abort_2_short_header);
> goto error;
> }
>
> - /* Extract the header from the skb */
> - hdr = data + offset;
> - offset += sizeof(*hdr);
> - len -= sizeof(*hdr);
> -
> if (ntohl(hdr->epoch) != call->conn->proto.epoch ||
> ntohl(hdr->cid) != call->cid ||
> ntohl(hdr->call_number) != call->call_id ||
> ntohl(hdr->seq) != sp->hdr.seq ||
> ntohl(hdr->sec_index) != call->security_ix ||
> - ntohl(hdr->data_len) > len) {
> + ntohl(hdr->data_len) > xdr.len) {
> ret = rxrpc_abort_eproto(call, skb, RXGK_SEALEDINCON,
> rxgk_abort_2_short_data);
> goto error;
> }
>
> - call->rx_dec_offset = offset;
> + call->rx_dec_offset = (void *)xdr.p - call->rx_dec_buffer;
> call->rx_dec_len = ntohl(hdr->data_len);
> ret = 0;
> error:
> @@ -1073,37 +1072,37 @@ 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,
> + struct xdr_buffer *auth)
> {
> - u32 app_len, call_count, level, epoch, cid, i;
> + struct xdr_buffer appdata;
> + xdr_t *nonce, *x;
> + u32 call_count, level, epoch, cid, i;
>
> _enter("");
>
> - if ((end - p) * sizeof(__be32) < 24)
> + nonce = xdr_extract_region(auth, 20);
> + if (!nonce)
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_short_auth);
> - if (memcmp(p, conn->rxgk.nonce, 20) != 0)
> + if (memcmp(nonce, conn->rxgk.nonce, 20) != 0)
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_bad_nonce);
> - p += 20 / sizeof(__be32);
>
> - app_len = ntohl(*p++);
> - if (app_len > (end - p) * sizeof(__be32))
> + if (!xdr_extract_blob(auth, &appdata))
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_short_applen);
>
> - p += xdr_round_up(app_len) / sizeof(__be32);
> - if (end - p < 4)
> + x = xdr_extract_region(auth, 4 * sizeof(xdr_t));
> + if (!x)
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_short_auth);
> -
> - level = ntohl(*p++);
> - epoch = ntohl(*p++);
> - cid = ntohl(*p++);
> - call_count = ntohl(*p++);
> + level = xdr_dec(x[0]);
> + epoch = xdr_dec(x[1]);
> + cid = xdr_dec(x[2]);
> + call_count = xdr_dec(x[3]);
>
> if (level != conn->security_level ||
> epoch != conn->proto.epoch ||
> @@ -1112,12 +1111,13 @@ static int rxgk_do_verify_authenticator(struct rxrpc_connection *conn,
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_bad_param);
>
> - if (end - p < call_count)
> + x = xdr_extract_region(auth, call_count * sizeof(xdr_t));
> + if (!x)
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> rxgk_abort_resp_short_call_list);
>
> for (i = 0; i < call_count; i++) {
> - u32 call_id = ntohl(*p++);
> + u32 call_id = xdr_dec(x[i]);
>
> if (call_id > INT_MAX)
> return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EPROTO,
> @@ -1140,37 +1140,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.
> *
> @@ -1181,55 +1150,34 @@ 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,
> + struct xdr_buffer *response)
> {
> 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 xdr_buffer resp_token, auth;
> 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;
> - __be32 xauth_len;
> int ret, ec;
>
> _enter("{%d}", conn->debug_id);
>
> /* Parse the RXGK_Response object */
> - if (sizeof(rhdr) + sizeof(__be32) > len)
> + rhdr = xdr_extract_region(response, sizeof(*rhdr));
> + if (!rhdr)
> goto short_packet;
> -
> - if (skb_copy_bits(skb, offset, &rhdr, sizeof(rhdr)) < 0)
> + if (!xdr_extract_blob(response, &resp_token))
> 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)
> + if (!xdr_extract_blob(response, &auth))
> 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);
> -
> - if (skb_copy_bits(skb, offset, &xauth_len, sizeof(xauth_len)) < 0)
> - goto short_packet;
> - offset += sizeof(xauth_len);
> - len -= sizeof(xauth_len);
> -
> - auth_offset = offset;
> - auth_len = ntohl(xauth_len);
> - if (auth_len > len)
> - goto short_packet;
> - if (auth_len & 3)
> + if (auth.len & 3)
> goto inconsistent;
> - if (auth_len < 20 + 9 * 4)
> + if (auth.len < 20 + 9 * 4)
> goto auth_too_short;
>
> /* We need to extract and decrypt the token and instantiate a session
> @@ -1238,7 +1186,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, &key);
> if (ret < 0)
> goto out;
>
> @@ -1252,7 +1200,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)) {
> @@ -1262,18 +1210,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, &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);
> if (ret < 0)
> goto out_gk;
>
> diff --git a/net/rxrpc/rxgk_app.c b/net/rxrpc/rxgk_app.c
> index 0ef2a29eb695..5ced22f17c5b 100644
> --- a/net/rxrpc/rxgk_app.c
> +++ b/net/rxrpc/rxgk_app.c
> @@ -40,40 +40,43 @@
> * };
> */
> int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int ticket_offset, unsigned int ticket_len,
> - struct key **_key)
> + struct xdr_buffer *xticket, struct key **_key)
> {
> struct rxrpc_key_token *token;
> const struct cred *cred = current_cred(); // TODO - use socket creds
> + struct xdr_buffer xdr = *xticket, K0;
> struct key *key;
> size_t pre_ticket_len, payload_len;
> - unsigned int klen, enctype;
> + unsigned int enctype;
> void *payload, *ticket;
> - __be32 *t, *p, *q, tmp[2];
> + __be32 *t, *p, *q;
> + xdr_t *x;
> int ret;
>
> _enter("");
>
> - if (ticket_len < 10 * sizeof(__be32))
> + /* Extract K0 */
> + x = xdr_extract_region(&xdr, sizeof(xdr_t) * 1);
> + if (!x)
> return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
> 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);
> - enctype = ntohl(tmp[0]);
> - klen = ntohl(tmp[1]);
> + enctype = xdr_dec(*x);
>
> - if (klen > ticket_len - 10 * sizeof(__be32))
> + if (!xdr_extract_blob(&xdr, &K0))
> return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
> rxgk_abort_resp_short_yfs_key);
>
> + /* Extract level to expirationtime. */
> + t = xdr_extract_region(&xdr, sizeof(xdr_t) * 7);
> + if (!t)
> + return rxrpc_abort_conn(conn, skb, RXGK_INCONSISTENCY, -EPROTO,
> + rxgk_abort_resp_short_yfs_tkt);
> +
> pre_ticket_len = ((5 + 14) * sizeof(__be32) +
> - xdr_round_up(klen) +
> + xdr_round_up(K0.len) +
> sizeof(__be32));
> - payload_len = pre_ticket_len + xdr_round_up(ticket_len);
> + payload_len = pre_ticket_len + xdr_round_up(xticket->len);
>
> payload = kzalloc(payload_len, GFP_NOFS);
> if (!payload)
> @@ -84,12 +87,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, xticket->p, xticket->len);
>
> /* Fill out the form header. */
> p = payload;
> @@ -97,13 +95,12 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> p[1] = htonl(1); /* len(cellname) */
> p[2] = htonl(0x20000000); /* Cellname " " */
> p[3] = htonl(1); /* #tokens */
> - p[4] = htonl(15 * sizeof(__be32) + xdr_round_up(klen) +
> - xdr_round_up(ticket_len)); /* Token len */
> + p[4] = htonl(15 * sizeof(__be32) + xdr_round_up(K0.len) +
> + xdr_round_up(xticket->len)); /* Token len */
>
> /* Now fill in the body. Most of this we can just scrape directly from
> * the ticket.
> */
> - t = ticket + sizeof(__be32) * 2 + xdr_round_up(klen);
> q = payload + 5 * sizeof(__be32);
> q[0] = htonl(RXRPC_SECURITY_YFS_RXGK);
> q[1] = t[1]; /* begintime - msw */
> @@ -118,21 +115,21 @@ int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> q[10] = t[4]; /* - lsw */
> q[11] = 0; /* enctype - msw */
> q[12] = htonl(enctype); /* - lsw */
> - q[13] = htonl(klen); /* Key length */
> + q[13] = htonl(K0.len); /* Key length */
>
> q += 14;
>
> - memcpy(q, ticket + sizeof(__be32) * 2, klen);
> - q += xdr_round_up(klen) / 4;
> - q[0] = htonl(ticket_len);
> + memcpy(q, K0.p, K0.len);
> + q += xdr_round_up(K0.len) / 4;
> + q[0] = htonl(xticket->len);
> q++;
> if (WARN_ON((unsigned long)q != (unsigned long)ticket)) {
> ret = -EIO;
> goto error;
> }
>
> - /* Ticket read in with skb_copy_bits above */
> - q += xdr_round_up(ticket_len) / 4;
> + /* Ticket appended above. */
> + q += xdr_round_up(xticket->len) / 4;
> if (WARN_ON((unsigned long)q - (unsigned long)payload != payload_len)) {
> ret = -EIO;
> goto error;
> @@ -182,44 +179,34 @@ 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,
> - struct key **_key)
> + struct xdr_buffer *token, struct key **_key)
> {
> const struct krb5_enctype *krb5;
> const struct krb5_buffer *server_secret;
> struct crypto_aead *token_enc = NULL;
> + struct xdr_buffer ticket;
> struct key *server_key;
> - unsigned int ticket_offset, ticket_len;
> u32 kvno, enctype;
> int ret, ec = 0;
>
> struct {
> __be32 kvno;
> __be32 enctype;
> - __be32 token_len;
> - } container;
> + } *container;
>
> - if (token_len < sizeof(container))
> + container = xdr_extract_region(token, sizeof(container));
> + if (!container)
> goto short_packet;
>
> /* Decode the RXGK_TokenContainer object. This tells us which server
> * 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;
> -
> - kvno = ntohl(container.kvno);
> - enctype = ntohl(container.enctype);
> - ticket_len = ntohl(container.token_len);
> - ticket_offset = token_offset + sizeof(container);
> -
> - if (ticket_len > xdr_round_down(token_len - sizeof(container)))
> - goto short_packet;
> + kvno = ntohl(container->kvno);
> + enctype = ntohl(container->enctype);
>
> _debug("KVNO %u", kvno);
> _debug("ENC %u", enctype);
> - _debug("TLEN %u", ticket_len);
>
> server_key = rxrpc_look_up_server_security(conn, skb, kvno, enctype);
> if (IS_ERR(server_key))
> @@ -237,8 +224,11 @@ 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);
> + if (!xdr_extract_blob(token, &ticket))
> + goto short_packet;
> + _debug("TLEN %u", ticket.len);
> +
> + ret = rxgk_decrypt(krb5, token_enc, &ticket, &ec);
> crypto_free_aead(token_enc);
> token_enc = NULL;
> if (ret < 0) {
> @@ -248,8 +238,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,
> - ticket_len, _key);
> + ret = conn->security->default_decode_ticket(conn, skb, &ticket, _key);
> if (ret < 0)
> goto cant_get_token;
>
> diff --git a/net/rxrpc/rxgk_common.h b/net/rxrpc/rxgk_common.h
> index dc8b0f106104..f0b724c44036 100644
> --- a/net/rxrpc/rxgk_common.h
> +++ b/net/rxrpc/rxgk_common.h
> @@ -33,19 +33,13 @@ struct rxgk_context {
> struct crypto_aead *resp_enc; /* Response packet enc key */
> };
>
> -#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))
> -
> /*
> * rxgk_app.c
> */
> int rxgk_yfs_decode_ticket(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int ticket_offset, unsigned int ticket_len,
> - struct key **_key);
> + struct xdr_buffer *xticket, struct key **_key);
> int rxgk_extract_token(struct rxrpc_connection *conn, struct sk_buff *skb,
> - unsigned int token_offset, unsigned int token_len,
> - struct key **_key);
> + struct xdr_buffer *token, struct key **_key);
>
> /*
> * rxgk_kdf.c
> @@ -61,50 +55,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 offset
> * and length are updated to reflect the actual content of the encrypted
> @@ -112,21 +62,24 @@ int rxgk_decrypt_skb(const struct krb5_enctype *krb5,
> */
> static inline int rxgk_decrypt(const struct krb5_enctype *krb5,
> struct crypto_aead *aead,
> - void *data,
> - unsigned int *_offset, unsigned int *_len,
> + struct xdr_buffer *buf,
> int *_error_code)
> {
> struct scatterlist sg[1];
> - size_t offset = 0, len = *_len;
> + size_t offset = 0, len = buf->len;
> int ret;
>
> - sg_init_one(sg, data, len);
> -
> + sg_init_one(sg, buf->p, len);
> ret = crypto_krb5_decrypt(krb5, aead, sg, 1, &offset, &len);
> switch (ret) {
> case 0:
> - *_offset += offset;
> - *_len = len;
> + if (offset & 3) {
> + *_error_code = RXGK_INCONSISTENCY;
> + ret = -EPROTO;
> + break;
> + }
> + buf->p = (void *)buf->p + offset;
> + buf->len = len;
> break;
> case -EBADMSG: /* Checksum mismatch. */
> case -EPROTO:
> @@ -144,50 +97,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 offset 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..e0f3db451b05 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,20 +1111,49 @@ 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,
> + struct xdr_buffer *response_xdr)
> {
> 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;
>
> _enter("{%d}", conn->debug_id);
>
> + response = xdr_extract_region(response_xdr, sizeof(*response));
> + if (!response)
> + return rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> + rxkad_abort_resp_short);
> +
> + version = ntohl(response->version);
> + ticket_len = ntohl(response->ticket_len);
> + kvno = ntohl(response->kvno);
> +
> + trace_rxrpc_rx_response(conn, sp->hdr.serial, version, kvno, ticket_len);
> +
> + if (version != RXKAD_VERSION)
> + return rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
> + rxkad_abort_resp_version);
> +
> + if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5)
> + return rxrpc_abort_conn(conn, skb, RXKADUNKNOWNKEY, -EPROTO,
> + rxkad_abort_resp_unknown_tkt);
> +
> + ticket = xdr_extract_region(response_xdr, ticket_len);
> + if (!ticket)
> + return rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> + rxkad_abort_resp_short_tkt);
> +
> + if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN)
> + return rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
> + rxkad_abort_resp_tkt_len);
> +
> server_key = rxrpc_look_up_server_security(conn, skb, 0, 0);
> if (IS_ERR(server_key)) {
> ret = PTR_ERR(server_key);
> @@ -1142,55 +1170,10 @@ 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) {
> - ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> - rxkad_abort_resp_short);
> - goto error;
> - }
> -
> - version = ntohl(response->version);
> - ticket_len = ntohl(response->ticket_len);
> - kvno = ntohl(response->kvno);
> -
> - trace_rxrpc_rx_response(conn, sp->hdr.serial, version, kvno, ticket_len);
> -
> - if (version != RXKAD_VERSION) {
> - ret = rxrpc_abort_conn(conn, skb, RXKADINCONSISTENCY, -EPROTO,
> - rxkad_abort_resp_version);
> - goto error;
> - }
> -
> - if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN) {
> - ret = rxrpc_abort_conn(conn, skb, RXKADTICKETLEN, -EPROTO,
> - rxkad_abort_resp_tkt_len);
> - goto error;
> - }
> -
> - if (kvno >= RXKAD_TKT_TYPE_KERBEROS_V5) {
> - 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 error;
> -
> - if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header) + sizeof(*response),
> - ticket, ticket_len) < 0) {
> - ret = rxrpc_abort_conn(conn, skb, RXKADPACKETSHORT, -EPROTO,
> - rxkad_abort_resp_short_tkt);
> - goto error;
> - }
> -
> + /* Extract the kerberos ticket and decrypt and decode it. We copy it
> + * before decryption which ensures it's correctly aligned for the
> + * decryption.
> + */
> ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
> &session_key, &expiry);
> if (ret < 0)
> @@ -1265,8 +1248,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;
>
It would be easier to review this change if the use of a temporary buffer was
separated from the XDR abstraction changes. Would it be possible to do so?
Thank you.
Jeffrey Altman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4120 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-afs/attachments/20260512/f3a9f050/attachment-0001.p7s>
More information about the linux-afs
mailing list