Thread (28 messages) 28 messages, 4 authors, 12d ago

Re: [PATCH net v10 10/14] rxrpc: Fix RxGK key parser to check enctype is supported

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-15 22:57:44
Also in: lkml

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] rxrpc_preparse_xdr_yfs_rxgk() charges the full token cost to
  prep->quotalen at net/rxrpc/key.c:209 (`prep->quotalen += datalen +…

--

rxrpc: Fix RxGK key parser to check enctype is supported

The RxGK key parser now looks up the userspace-supplied encryption type
with crypto_krb5_find_enctype() and rejects the token if the enctype is
unknown or if the supplied key length does not match the enctype.  The
RxGK-specific token handling is also made conditional on CONFIG_RXGK.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
quoted hunk ↗ jump to hunk
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index cbd26da449510..cdef609996d28 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -166,12 +167,14 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 				       size_t datalen,
 				       const __be32 *xdr, unsigned int toklen)
 {
+	const struct krb5_enctype *enc;
 	struct rxrpc_key_token *token, **pptoken;
 	time64_t expiry;
 	size_t plen;
 	const __be32 *ticket, *key;
 	s64 tmp;
 	size_t raw_keylen, raw_tktlen, keylen, tktlen;
+	int ret = -EKEYREJECTED;
 
 	_enter(",{%x,%x,%x,%x},%x",
 	       ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
[ ... ]
quoted hunk ↗ jump to hunk
@@ -229,6 +232,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
 	token->rxgk->key.data	= token->rxgk->_key;
 	token->rxgk->ticket.len = raw_tktlen;
 
+	/* Check the enctype is supported. */
+	enc = crypto_krb5_find_enctype(token->rxgk->enctype);
+	if (!enc) {
+		ret = -ENOPKG;
+		goto reject_token;
+	}
[Severity: Medium]
Can this new -ENOPKG path leave prep->quotalen charged for memory that
was freed?

Earlier in rxrpc_preparse_xdr_yfs_rxgk() the whole token cost is added to
the quota before any of these checks run:

	plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
	prep->quotalen += datalen + plen;

with tktlen bounded only by AFSTOKEN_GK_TOKEN_MAX (16384).  The
reject_token label then does kfree(token->rxgk) and kfree(token) without
subtracting that amount again.

Every pre-existing reject path here returned -EKEYREJECTED, -EKEYEXPIRED
or -ENOMEM, and rxrpc_preparse_xdr() turns those into an abort:

		switch (ret2) {
		case 0:
			ret = 0;
			break;
		case -EPROTONOSUPPORT:
			break;
		case -ENOPKG:
			if (ret != 0)
				ret = -ENOPKG;
			break;
		default:
			ret = ret2;
			goto error;
		}

-ENOPKG is the soft case: if another token in the same payload (for
example an RXKAD token) parsed successfully, ret stays 0 and the key is
still instantiated, so __key_instantiate_and_link() ->
key_payload_reserve(key, prep->quotalen) commits the inflated value
against the owner's key quota:

security/keys/key.c:key_payload_reserve() {
	...
		if (delta > 0 &&
		    (key->user->qnbytes + delta > maxbytes ||
		     key->user->qnbytes + delta < key->user->qnbytes)) {
			ret = -EDQUOT;
	...
}

Since key_quota_maxbytes defaults to 20000 bytes, a single crafted
add_key("rxrpc", "afs@...", ...) payload containing one valid RXKAD token
plus an RxGK token with an unsupported enctype appears to be able to
charge roughly 16KB that is not actually held, making later add_key()
calls for that uid fail with -EDQUOT.

Would it be better to do the enctype lookup and key length check before
the prep->quotalen += datalen + plen accounting, or to subtract
datalen + plen on the -ENOPKG path?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help