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

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

[Lukas Wunner]

[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

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

[Jarkko Sakkinen]

On Tue, Jun 23, 2026 at 11:26:05AM +0200, Lukas Wunner wrote:
> [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

Lukas, thank you for forwarding this, and with a quick sim I do.

Can this be next time be also CC'd to keyrings@vger.kernel.org, in
addition of me and rest? What I can then do is to look this in detail,
test the change, apply and eventually forward to Linus...

BR, Jarkko