All of lore.kernel.org
 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 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.