* [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.