Linux Security Modules development
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Daehyeon Ko <4ncienth@gmail.com>
Cc: David Howells <dhowells@redhat.com>,
	Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Jason A . Donenfeld" <Jason@zx2c4.com>,
	keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] keys: big_key: Reserve the final quota before destroying the payload
Date: Sat, 17 Oct 2026 02:40:35 +0300	[thread overview]
Message-ID: <atK186hMPBhYyPCN@kernel.org> (raw)
In-Reply-To: <atK0MnGM2h9IH6FD@kernel.org>

On Sat, Oct 17, 2026 at 02:33:10AM +0300, Jarkko Sakkinen wrote:
> On Fri, Oct 16, 2026 at 01:00:04PM +0900, Daehyeon Ko wrote:
> > A big_key is charged a fixed 16 bytes of key quota, regardless of the
> > actual payload size.  big_key_update() currently reserves the actual new
> > payload length before destroying the old payload, while the subsequent
> > generic_key_instantiate() reserves the fixed quota length.
> > 
> > For an update to a payload shorter than 16 bytes, the first reservation
> > temporarily returns quota.  Another key operation by the same user can
> > consume that quota before generic_key_instantiate() tries to reserve it
> > again.  If that second reservation fails, the key remains instantiated but
> > its old big_key payload has already been destroyed.  Reading a
> > buffer-backed key or revoking a shmem-backed key then dereferences a NULL
> > payload pointer.
> 
> Even at the risk that I'm saying something stupid, makes me
> wonder why the call exists in big_key.c? I.e. why not remove
> the call instead of doing what the patch does?
> 
> If I'm now missing something out, that's cool, just correct
> me.

So the way I see it this patch has the right bug and a wrong fix.

I.e. if you still want to move forward with this fix, explain
why two repeated calls are still a necessity. Then we can ensure
that the fix addressing the root cause.

Alternatively rewrite the patch so that it makes the call path have
only a single call site for key_payload_reserve().

Br. Jarkko

      reply	other threads:[~2026-10-16 23:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  4:00 [PATCH] keys: big_key: Reserve the final quota before destroying the payload Daehyeon Ko
2026-10-16  4:07 ` sashiko-bot
2026-10-16 23:33 ` Jarkko Sakkinen
2026-10-16 23:40   ` Jarkko Sakkinen [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=atK186hMPBhYyPCN@kernel.org \
    --to=jarkko@kernel.org \
    --cc=4ncienth@gmail.com \
    --cc=Jason@zx2c4.com \
    --cc=dhowells@redhat.com \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=stable@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