[RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

Call key_put() before returning -EKEYREVOKED, as the reference count gets
incremented by key_lookup().

Cc: stable@vger.kernel.org # v6.12+
Fixes: 5bc46b49c828 ("nvme-tcp: check for invalidated or revoked key")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/nvme/common/keyring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
index 32d16c53133b..87cb51e1f5a7 100644
--- a/drivers/nvme/common/keyring.c
+++ b/drivers/nvme/common/keyring.c
@@ -35,6 +35,7 @@ struct key *nvme_tls_key_lookup(key_serial_t key_id)
 	}
 	if (nvme_tls_psk_revoked(key)) {
 		pr_err("key id %08x revoked\n", key_id);
+		key_put(key);
 		return ERR_PTR(-EKEYREVOKED);
 	}
 	return key;
-- 
2.47.3

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Christoph Hellwig]

On Tue, Nov 03, 2026 at 05:02:07AM +0200, Jarkko Sakkinen wrote:
> Call key_put() before returning -EKEYREVOKED, as the reference count gets
> incremented by key_lookup().

Looks good, but I only got this one patch out of a 11 patch series,
so without the full context it's hard to fully review it.

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

On Tue, Nov 03, 2026 at 06:33:03AM +0100, Christoph Hellwig wrote:
> On Tue, Nov 03, 2026 at 05:02:07AM +0200, Jarkko Sakkinen wrote:
> > Call key_put() before returning -EKEYREVOKED, as the reference count gets
> > incremented by key_lookup().
> 
> Looks good, but I only got this one patch out of a 11 patch series,
> so without the full context it's hard to fully review it.

I can it is just I rather do it on request because some people complain
when doing what you asked :-) Regardless, the bug is independent of any
patch set changes.

I'll summarize the context below (ignore or read).

I tried to tackle: https://lore.kernel.org/lkml/20261101221519.GB2689@quark/

And essentially what I would want is key_type_dead not to exist.

This version is based on module pinning, and  thus not having that
key_put() would keep key type module alive in that case. However, it
turned out this approach will not fly as AF_RXRPC and DNS resolver will
hit the wall given e.g., the remote pinnings.

The approach I'm going for next is to have a type registry instead,
which is independent of the module system (to put story really short).
I think I'm doing this with right process as this really needs a few
trials in order to find patterns that stick.

Br, Jarkko

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Hannes Reinecke]

On 11/3/26 4:02 AM, Jarkko Sakkinen wrote:
> Call key_put() before returning -EKEYREVOKED, as the reference count gets
> incremented by key_lookup().
> 
> Cc: stable@vger.kernel.org # v6.12+
> Fixes: 5bc46b49c828 ("nvme-tcp: check for invalidated or revoked key")
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>   drivers/nvme/common/keyring.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
> index 32d16c53133b..87cb51e1f5a7 100644
> --- a/drivers/nvme/common/keyring.c
> +++ b/drivers/nvme/common/keyring.c
> @@ -35,6 +35,7 @@ struct key *nvme_tls_key_lookup(key_serial_t key_id)
>   	}
>   	if (nvme_tls_psk_revoked(key)) {
>   		pr_err("key id %08x revoked\n", key_id);
> +		key_put(key);
>   		return ERR_PTR(-EKEYREVOKED);
>   	}
>   	return key;

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Christoph Hellwig]

On Tue, Nov 03, 2026 at 02:29:11PM +0200, Jarkko Sakkinen wrote:
> And essentially what I would want is key_type_dead not to exist.
> 
> This version is based on module pinning, and  thus not having that
> key_put() would keep key type module alive in that case. However, it
> turned out this approach will not fly as AF_RXRPC and DNS resolver will
> hit the wall given e.g., the remote pinnings.
> 
> The approach I'm going for next is to have a type registry instead,
> which is independent of the module system (to put story really short).
> I think I'm doing this with right process as this really needs a few
> trials in order to find patterns that stick.

So what's the plan here.  Should we merge this standalone as a fix,
or are you trying to get an entire series in?

If it's useful as a standalone fix, please resend it standalone.

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

On Wed, Nov 04, 2026 at 10:04:40AM +0100, Christoph Hellwig wrote:
> On Tue, Nov 03, 2026 at 02:29:11PM +0200, Jarkko Sakkinen wrote:
> > And essentially what I would want is key_type_dead not to exist.
> > 
> > This version is based on module pinning, and  thus not having that
> > key_put() would keep key type module alive in that case. However, it
> > turned out this approach will not fly as AF_RXRPC and DNS resolver will
> > hit the wall given e.g., the remote pinnings.
> > 
> > The approach I'm going for next is to have a type registry instead,
> > which is independent of the module system (to put story really short).
> > I think I'm doing this with right process as this really needs a few
> > trials in order to find patterns that stick.
> 
> So what's the plan here.  Should we merge this standalone as a fix,
> or are you trying to get an entire series in?
> 
> If it's useful as a standalone fix, please resend it standalone.

I'll send this as a standalone patch.

Br, Jarkko

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

On Wed, Nov 04, 2026 at 08:45:11AM +0100, Hannes Reinecke wrote:
> On 11/3/26 4:02 AM, Jarkko Sakkinen wrote:
> > Call key_put() before returning -EKEYREVOKED, as the reference count gets
> > incremented by key_lookup().
> > 
> > Cc: stable@vger.kernel.org # v6.12+
> > Fixes: 5bc46b49c828 ("nvme-tcp: check for invalidated or revoked key")
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >   drivers/nvme/common/keyring.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/nvme/common/keyring.c b/drivers/nvme/common/keyring.c
> > index 32d16c53133b..87cb51e1f5a7 100644
> > --- a/drivers/nvme/common/keyring.c
> > +++ b/drivers/nvme/common/keyring.c
> > @@ -35,6 +35,7 @@ struct key *nvme_tls_key_lookup(key_serial_t key_id)
> >   	}
> >   	if (nvme_tls_psk_revoked(key)) {
> >   		pr_err("key id %08x revoked\n", key_id);
> > +		key_put(key);
> >   		return ERR_PTR(-EKEYREVOKED);
> >   	}
> >   	return key;
> 
> Reviewed-by: Hannes Reinecke <hare@kernel.org>

Thank you.

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@suse.de                                +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Br, Jarkko

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Keith Busch]

On Wed, Nov 04, 2026 at 04:43:09PM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 04, 2026 at 10:04:40AM +0100, Christoph Hellwig wrote:
> > On Tue, Nov 03, 2026 at 02:29:11PM +0200, Jarkko Sakkinen wrote:
> > > And essentially what I would want is key_type_dead not to exist.
> > > 
> > > This version is based on module pinning, and  thus not having that
> > > key_put() would keep key type module alive in that case. However, it
> > > turned out this approach will not fly as AF_RXRPC and DNS resolver will
> > > hit the wall given e.g., the remote pinnings.
> > > 
> > > The approach I'm going for next is to have a type registry instead,
> > > which is independent of the module system (to put story really short).
> > > I think I'm doing this with right process as this really needs a few
> > > trials in order to find patterns that stick.
> > 
> > So what's the plan here.  Should we merge this standalone as a fix,
> > or are you trying to get an entire series in?
> > 
> > If it's useful as a standalone fix, please resend it standalone.
> 
> I'll send this as a standalone patch.

I can just apply it as-is unless you plan to make any change from what
was submitted. Just future reference that the intention wasn't clear
when a series' recipients are split up this way.

Re: (subset) [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Keith Busch]

On Tue, 3 Nov 2026 05:02:07 +0200, Jarkko Sakkinen wrote:
> Call key_put() before returning -EKEYREVOKED, as the reference count gets
> incremented by key_lookup().
> 
> 

Applied, thanks!

[02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()
        commit: 404a8f6caa63822c1d2a45adbf07fd9c0567a7ef

Best regards,
-- 
Keith Busch <kbusch@kernel.org>

Re: (subset) [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

On Wed, Nov 04, 2026 at 12:37:15PM -0800, Keith Busch wrote:
> 
> On Tue, 3 Nov 2026 05:02:07 +0200, Jarkko Sakkinen wrote:
> > Call key_put() before returning -EKEYREVOKED, as the reference count gets
> > incremented by key_lookup().
> > 
> > 
> 
> Applied, thanks!

Thank you and apologies for all this noise.

I'll send standalone patch next time if/when this happens.

> 
> [02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()
>         commit: 404a8f6caa63822c1d2a45adbf07fd9c0567a7ef
> 
> Best regards,
> -- 
> Keith Busch <kbusch@kernel.org>
> 


Br, Jarkko

Re: [RFC PATCH v2 02/11] nvme-keyring: Add a missing key_put() call to nvme_tls_key_lookup()

[Jarkko Sakkinen]

On Wed, Nov 04, 2026 at 08:53:04AM -0700, Keith Busch wrote:
> On Wed, Nov 04, 2026 at 04:43:09PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Nov 04, 2026 at 10:04:40AM +0100, Christoph Hellwig wrote:
> > > On Tue, Nov 03, 2026 at 02:29:11PM +0200, Jarkko Sakkinen wrote:
> > > > And essentially what I would want is key_type_dead not to exist.
> > > > 
> > > > This version is based on module pinning, and  thus not having that
> > > > key_put() would keep key type module alive in that case. However, it
> > > > turned out this approach will not fly as AF_RXRPC and DNS resolver will
> > > > hit the wall given e.g., the remote pinnings.
> > > > 
> > > > The approach I'm going for next is to have a type registry instead,
> > > > which is independent of the module system (to put story really short).
> > > > I think I'm doing this with right process as this really needs a few
> > > > trials in order to find patterns that stick.
> > > 
> > > So what's the plan here.  Should we merge this standalone as a fix,
> > > or are you trying to get an entire series in?
> > > 
> > > If it's useful as a standalone fix, please resend it standalone.
> > 
> > I'll send this as a standalone patch.
> 
> I can just apply it as-is unless you plan to make any change from what
> was submitted. Just future reference that the intention wasn't clear
> when a series' recipients are split up this way.

Yeah, I 100% agree with this.

Br, Jarkko