Linux cryptographic layer development
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: azraelxuemo <eilaimemedsnaimel@gmail.com>
Cc: linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au,
	dhowells@redhat.com, Ignat Korchagin <ignat@linux.win>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	keyrings@vger.kernel.org
Subject: Re: [PATCH] KEYS: asymmetric: fix OOB read in KEYCTL_PKEY_DECRYPT on zero-length message
Date: Tue, 23 Jun 2026 11:26:05 +0200	[thread overview]
Message-ID: <ajpRLY4unsqxS46e@wunner.de> (raw)
In-Reply-To: <20260622025002.798934-1-xuemo@xuemo.com>

[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

      reply	other threads:[~2026-06-23  9:26 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajpRLY4unsqxS46e@wunner.de \
    --to=lukas@wunner.de \
    --cc=dhowells@redhat.com \
    --cc=eilaimemedsnaimel@gmail.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=ignat@linux.win \
    --cc=jarkko@kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox