Re: [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message

Lukas Wunner <[email protected]> Tue, 23 Jun 2026 11:26:05 +0200
Newsgroups org.kernel.vger.keyrings,org.kernel.vger.linux-crypto
Message-ID <[email protected]>
[cc += Ignat, Jarkko, keyrings; start of thread is here:
https://lore.kernel.org/r/[email protected]
]

On Mon, Jun 22, 2026 at 02:50:02AM +0000, azraelxuemo wrote:
> When ret is replaced with maxsize, the caller keyctl_pkey_e_d_s()
> does copy_to_user(_out, out, ret) with ret = key_size (e.g. 256
> for RSA-2048) on a buffer allocated with kmalloc(params.out_len),
> which can be as small as 1 byte.  This reads key_size - out_len
> bytes beyond the allocation.

It would probably make sense to tighten security in keyctl_pkey_e_d_s()
by using kzalloc() instead of kmalloc() and by capping the amount of
data copied with min(ret, params.out_len).

> Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> Signed-off-by: HanQuan <[email protected]>

Please add:

Cc: [email protected] # v6.5+

You don't need to cc that address when submitting the patch,
but including the tag in the commit message helps stable
maintainers identify patches that need backporting.

> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -358,7 +358,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
>  		BUG();
>  	}
>  
> -	if (!issig && ret == 0)
> +	/* Decrypt may legitimately return 0 (zero-length message); only
> +	 * replace ret with maxsize for encrypt, which returns 0 on success.
> +	 */
> +	if (!issig && ret == 0 && params->op == kernel_pkey_encrypt)
>  		ret = crypto_akcipher_maxsize(tfm);

Given that out of 3 operations (encrypt, decrypt, sign),
2 already return the size, I think a better approach would be
to let crypto_akcipher_sync_encrypt() return crypto_akcipher_maxsize()
on success, i.e.:

	return crypto_akcipher_sync_prep(&data) ?:
	       crypto_akcipher_sync_post(&data,
-					 crypto_akcipher_encrypt(data.req));
+					 crypto_akcipher_encrypt(data.req)) ?:
+	       crypto_akcipher_maxsize(tfm);

and then remove the if-clause in software_key_eds_op() altogether
which overwrites ret with the maxsize.

Do you agree?

Your patch wasn't cc'ed to all maintainers of this file.
Please double-check that you've checked out Linus' current
master when running scripts/get_maintainer.pl.

Thanks,

Lukas