All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message
@ 2026-06-22  2:50 azraelxuemo
  2026-06-23  9:26 ` Lukas Wunner
  0 siblings, 1 reply; 2+ messages in thread
From: azraelxuemo @ 2026-06-22  2:50 UTC (permalink / raw)
  To: linux-crypto; +Cc: herbert, dhowells, HanQuan

From: HanQuan <eilaimemedsnaimel@gmail.com>

In software_key_eds_op(), the condition

    if (!issig && ret == 0)
        ret = crypto_akcipher_maxsize(tfm);

replaces ret with the key size when crypto_akcipher_sync_decrypt()
returns 0.  This was added as a workaround for encrypt, where
crypto_akcipher_sync_encrypt() returns 0 on success (it lacks the
"?:" data.dlen" tail that sync_decrypt has).  However, for decrypt,
ret == 0 is legitimate: pkcs1pad_decrypt_complete() sets
req->dst_len = 0 for a zero-length PKCS#1 message, causing
crypto_akcipher_sync_decrypt() to return 0 via "0 ?: data.dlen".

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.

Restrict the maxsize substitution to the encrypt operation only,
where ret == 0 genuinely indicates success and the full key size
is the correct output length.

Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
Signed-off-by: HanQuan <eilaimemedsnaimel@gmail.com>
---
 crypto/asymmetric_keys/public_key.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 09a0b83d5d77..2c3ac75ec92b 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ 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);
 
 error_free_tfm:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message
  2026-06-22  2:50 [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message azraelxuemo
@ 2026-06-23  9:26 ` Lukas Wunner
  0 siblings, 0 replies; 2+ messages in thread
From: Lukas Wunner @ 2026-06-23  9:26 UTC (permalink / raw)
  To: azraelxuemo
  Cc: linux-crypto, herbert, dhowells, Ignat Korchagin, Jarkko Sakkinen,
	keyrings

[cc += Ignat, Jarkko, keyrings; start of thread is here:
https://lore.kernel.org/r/20260622025002.798934-1-xuemo@xuemo.com
]

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 <eilaimemedsnaimel@gmail.com>

Please add:

Cc: stable@vger.kernel.org # 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-23  9:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22  2:50 [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message azraelxuemo
2026-06-23  9:26 ` Lukas Wunner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.