public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] rxrpc: Fix key and keyring reference count leaks
@ 2026-03-13 13:23 Anderson Nascimento
  2026-03-13 13:23 ` [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt() Anderson Nascimento
  2026-03-13 13:23 ` [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call() Anderson Nascimento
  0 siblings, 2 replies; 10+ messages in thread
From: Anderson Nascimento @ 2026-03-13 13:23 UTC (permalink / raw)
  To: dhowells
  Cc: marc.dionne, davem, edumazet, kuba, pabeni, horms, linux-afs,
	netdev, linux-kernel, Anderson Nascimento

Hello,

While auditing the RxRPC protocol, I identified two separate reference count leaks related to security keys and keyrings.

The first leak occurs during client call allocation if security initialization fails. The second occurs in the setsockopt path due to an incorrect struct member check, allowing multiple keyring assignments to the same socket. Both issues prevent the cleanup of key/keyring objects, as evidenced by /proc/keys remaining populated after the user processes exit.

This series fixes both issues by ensuring key_put() is called on the error path in the call allocator and by correcting the logic in rxrpc_setsockopt().

Patch Summary:

rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()

Prevents multiple keyring assignments to a single socket by checking rx->securities instead of rx->key.

rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()

Releases the key reference if rxrpc_init_client_call_security() fails.

Testing was performed by monitoring /proc/keys and using a reproducer that triggers failed security initialization and repeated setsockopt calls.

Anderson Nascimento (2):
  rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()

 net/rxrpc/af_rxrpc.c    | 2 +-
 net/rxrpc/call_object.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  2026-03-13 13:23 [PATCH 0/2] rxrpc: Fix key and keyring reference count leaks Anderson Nascimento
@ 2026-03-13 13:23 ` Anderson Nascimento
  2026-03-19 16:10   ` Simon Horman
  2026-03-13 13:23 ` [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call() Anderson Nascimento
  1 sibling, 1 reply; 10+ messages in thread
From: Anderson Nascimento @ 2026-03-13 13:23 UTC (permalink / raw)
  To: dhowells
  Cc: marc.dionne, davem, edumazet, kuba, pabeni, horms, linux-afs,
	netdev, linux-kernel, Anderson Nascimento

In rxrpc_setsockopt(), the code checks 'rx->key' when handling the
RXRPC_SECURITY_KEYRING option. However, this appears to be a logic error.
The code should be checking 'rx->securities' to determine if a keyring
has already been defined for the socket.

Currently, if a user calls setsockopt(RXRPC_SECURITY_KEYRING) multiple
times on the same socket, the check 'if (rx->key)' fails to block
subsequent calls because 'rx->key' has not been defined by the function.
This results in a reference count leak on the keyring.

This patch changes the check to 'rx->securities' to correctly identify
if the socket security keyring has already been configured, returning -EINVAL
on subsequent attempts.

Before the patch:

It shows the keyring reference counter elevated.

$ cat /proc/keys | grep AFSkeys1
27aca8ae I--Q--- 24469721 perm 3f010000  1000  1000 keyring   AFSkeys1: empty
$

After the patch:

The keyring reference counter remains stable and subsequent calls return an error:

$ ./poc
setsockopt: Invalid argument
$

Fixes: 17926a7 ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
---
 net/rxrpc/af_rxrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 0c2c68c4b07e..add72ac61f73 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -663,7 +663,7 @@ static int rxrpc_setsockopt(struct socket *sock, int level, int optname,
 
 		case RXRPC_SECURITY_KEYRING:
 			ret = -EINVAL;
-			if (rx->key)
+			if (rx->securities)
 				goto error;
 			ret = -EISCONN;
 			if (rx->sk.sk_state != RXRPC_UNBOUND)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()
  2026-03-13 13:23 [PATCH 0/2] rxrpc: Fix key and keyring reference count leaks Anderson Nascimento
  2026-03-13 13:23 ` [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt() Anderson Nascimento
@ 2026-03-13 13:23 ` Anderson Nascimento
  2026-03-18 21:46   ` David Howells
  2026-03-18 22:20   ` David Howells
  1 sibling, 2 replies; 10+ messages in thread
From: Anderson Nascimento @ 2026-03-13 13:23 UTC (permalink / raw)
  To: dhowells
  Cc: marc.dionne, davem, edumazet, kuba, pabeni, horms, linux-afs,
	netdev, linux-kernel, Anderson Nascimento

When creating a client call in rxrpc_alloc_client_call(), the code obtains a
reference to the key. If rxrpc_init_client_call_security() fails (e.g., due
to key invalidation), the function returns an error and destroys the call,
but fails to release the reference to the key.

This leads to a reference count leak, preventing the key from being
garbage collected.

Before the patch

It shows the key reference counter elevated. 

$ cat /proc/keys | grep afs@54321
1bffe9cd I--Q--i 8053480 4169w 3b010000  1000  1000 rxrpc     afs@54321: ka
$

After the patch

The invalidated key is removed when the code exits

$ cat /proc/keys | grep afs@54321
$

Fixes: f3441d4 ("rxrpc: Copy client call parameters into rxrpc_call earlier")
Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
---
 net/rxrpc/call_object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 918f41d97a2f..fbfcc611a2c4 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -227,6 +227,7 @@ static struct rxrpc_call *rxrpc_alloc_client_call(struct rxrpc_sock *rx,
 
 	ret = rxrpc_init_client_call_security(call);
 	if (ret < 0) {
+		key_put(call->key);
 		rxrpc_prefail_call(call, RXRPC_CALL_LOCAL_ERROR, ret);
 		rxrpc_put_call(call, rxrpc_call_put_discard_error);
 		return ERR_PTR(ret);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()
  2026-03-13 13:23 ` [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call() Anderson Nascimento
@ 2026-03-18 21:46   ` David Howells
  2026-03-18 22:20   ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: David Howells @ 2026-03-18 21:46 UTC (permalink / raw)
  To: Anderson Nascimento
  Cc: dhowells, marc.dionne, davem, edumazet, kuba, pabeni, horms,
	linux-afs, netdev, linux-kernel

Anderson Nascimento <anderson@allelesecurity.com> wrote:

> @@ -227,6 +227,7 @@ static struct rxrpc_call *rxrpc_alloc_client_call(struct rxrpc_sock *rx,
>  
>  	ret = rxrpc_init_client_call_security(call);
>  	if (ret < 0) {
> +		key_put(call->key);
>  		rxrpc_prefail_call(call, RXRPC_CALL_LOCAL_ERROR, ret);
>  		rxrpc_put_call(call, rxrpc_call_put_discard_error);
>  		return ERR_PTR(ret);

It's probably better to do this in rxrpc_destroy_call() if we can as a last
fallback.  In fact it's probably always leaking the call->key, not just under
this circumstance.

David


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()
  2026-03-13 13:23 ` [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call() Anderson Nascimento
  2026-03-18 21:46   ` David Howells
@ 2026-03-18 22:20   ` David Howells
  2026-03-18 22:30     ` Anderson Nascimento
  2026-03-19 14:46     ` Anderson Nascimento
  1 sibling, 2 replies; 10+ messages in thread
From: David Howells @ 2026-03-18 22:20 UTC (permalink / raw)
  To: Anderson Nascimento
  Cc: dhowells, marc.dionne, davem, edumazet, kuba, pabeni, horms,
	linux-afs, netdev, linux-kernel

Hi Anderson,

I think the patch can be done better as the attached - and this takes care of
another leak also.  Can you recheck your test?

Thanks,
David
---
commit 8e931ee13f267b814c0b668e9f52867b5239fed6
Author: Anderson Nascimento <anderson@allelesecurity.com>
Date:   Fri Mar 13 10:23:27 2026 -0300

    rxrpc: Fix key reference count leak from call->key
    
    When creating a client call in rxrpc_alloc_client_call(), the code obtains
    a reference to the key.  This is never cleaned up and gets leaked when the
    call is destroyed.
    
    Fix this by freeing call->key in rxrpc_destroy_call().
    
    Before the patch, it shows the key reference counter elevated:
    
    $ cat /proc/keys | grep afs@54321
    1bffe9cd I--Q--i 8053480 4169w 3b010000  1000  1000 rxrpc     afs@54321: ka
    $
    
    After the patch, the invalidated key is removed when the code exits:
    
    $ cat /proc/keys | grep afs@54321
    $
    
    Fixes: f3441d4125fc ("rxrpc: Copy client call parameters into rxrpc_call earlier")
    Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
    Co-developed-by: David Howells <dhowells@redhat.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    cc: Marc Dionne <marc.dionne@auristor.com>
    cc: Eric Dumazet <edumazet@google.com>
    cc: "David S. Miller" <davem@davemloft.net>
    cc: Jakub Kicinski <kuba@kernel.org>
    cc: Paolo Abeni <pabeni@redhat.com>
    cc: Simon Horman <horms@kernel.org>
    cc: linux-afs@lists.infradead.org
    cc: netdev@vger.kernel.org
    cc: stable@kernel.org

diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 918f41d97a2f..8d874ea428ff 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -694,6 +694,7 @@ static void rxrpc_destroy_call(struct work_struct *work)
 	rxrpc_put_bundle(call->bundle, rxrpc_bundle_put_call);
 	rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
 	rxrpc_put_local(call->local, rxrpc_local_put_call);
+	key_put(call->key);
 	call_rcu(&call->rcu, rxrpc_rcu_free_call);
 }
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()
  2026-03-18 22:20   ` David Howells
@ 2026-03-18 22:30     ` Anderson Nascimento
  2026-03-19 14:46     ` Anderson Nascimento
  1 sibling, 0 replies; 10+ messages in thread
From: Anderson Nascimento @ 2026-03-18 22:30 UTC (permalink / raw)
  To: David Howells
  Cc: marc.dionne, davem, edumazet, kuba, pabeni, horms, linux-afs,
	netdev, linux-kernel

Hi David,

That sounds better. I will test it and let you know. Thanks.

On Wed, Mar 18, 2026 at 7:20 PM David Howells <dhowells@redhat.com> wrote:
>
> Hi Anderson,
>
> I think the patch can be done better as the attached - and this takes care of
> another leak also.  Can you recheck your test?
>
> Thanks,
> David
> ---
> commit 8e931ee13f267b814c0b668e9f52867b5239fed6
> Author: Anderson Nascimento <anderson@allelesecurity.com>
> Date:   Fri Mar 13 10:23:27 2026 -0300
>
>     rxrpc: Fix key reference count leak from call->key
>
>     When creating a client call in rxrpc_alloc_client_call(), the code obtains
>     a reference to the key.  This is never cleaned up and gets leaked when the
>     call is destroyed.
>
>     Fix this by freeing call->key in rxrpc_destroy_call().
>
>     Before the patch, it shows the key reference counter elevated:
>
>     $ cat /proc/keys | grep afs@54321
>     1bffe9cd I--Q--i 8053480 4169w 3b010000  1000  1000 rxrpc     afs@54321: ka
>     $
>
>     After the patch, the invalidated key is removed when the code exits:
>
>     $ cat /proc/keys | grep afs@54321
>     $
>
>     Fixes: f3441d4125fc ("rxrpc: Copy client call parameters into rxrpc_call earlier")
>     Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
>     Co-developed-by: David Howells <dhowells@redhat.com>
>     Signed-off-by: David Howells <dhowells@redhat.com>
>     cc: Marc Dionne <marc.dionne@auristor.com>
>     cc: Eric Dumazet <edumazet@google.com>
>     cc: "David S. Miller" <davem@davemloft.net>
>     cc: Jakub Kicinski <kuba@kernel.org>
>     cc: Paolo Abeni <pabeni@redhat.com>
>     cc: Simon Horman <horms@kernel.org>
>     cc: linux-afs@lists.infradead.org
>     cc: netdev@vger.kernel.org
>     cc: stable@kernel.org
>
> diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
> index 918f41d97a2f..8d874ea428ff 100644
> --- a/net/rxrpc/call_object.c
> +++ b/net/rxrpc/call_object.c
> @@ -694,6 +694,7 @@ static void rxrpc_destroy_call(struct work_struct *work)
>         rxrpc_put_bundle(call->bundle, rxrpc_bundle_put_call);
>         rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
>         rxrpc_put_local(call->local, rxrpc_local_put_call);
> +       key_put(call->key);
>         call_rcu(&call->rcu, rxrpc_rcu_free_call);
>  }
>
>


-- 
Anderson Nascimento
Allele Security Intelligence
https://www.allelesecurity.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call()
  2026-03-18 22:20   ` David Howells
  2026-03-18 22:30     ` Anderson Nascimento
@ 2026-03-19 14:46     ` Anderson Nascimento
  1 sibling, 0 replies; 10+ messages in thread
From: Anderson Nascimento @ 2026-03-19 14:46 UTC (permalink / raw)
  To: David Howells
  Cc: marc.dionne, davem, edumazet, kuba, pabeni, horms, linux-afs,
	netdev, linux-kernel

On Wed, Mar 18, 2026 at 7:20 PM David Howells <dhowells@redhat.com> wrote:
>
> Hi Anderson,
>
> I think the patch can be done better as the attached - and this takes care of
> another leak also.  Can you recheck your test?

It fixes the issue. You can proceed with the patch. Thank you.

>
> Thanks,
> David
> ---
> commit 8e931ee13f267b814c0b668e9f52867b5239fed6
> Author: Anderson Nascimento <anderson@allelesecurity.com>
> Date:   Fri Mar 13 10:23:27 2026 -0300
>
>     rxrpc: Fix key reference count leak from call->key
>
>     When creating a client call in rxrpc_alloc_client_call(), the code obtains
>     a reference to the key.  This is never cleaned up and gets leaked when the
>     call is destroyed.
>
>     Fix this by freeing call->key in rxrpc_destroy_call().
>
>     Before the patch, it shows the key reference counter elevated:
>
>     $ cat /proc/keys | grep afs@54321
>     1bffe9cd I--Q--i 8053480 4169w 3b010000  1000  1000 rxrpc     afs@54321: ka
>     $
>
>     After the patch, the invalidated key is removed when the code exits:
>
>     $ cat /proc/keys | grep afs@54321
>     $
>
>     Fixes: f3441d4125fc ("rxrpc: Copy client call parameters into rxrpc_call earlier")
>     Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>
>     Co-developed-by: David Howells <dhowells@redhat.com>
>     Signed-off-by: David Howells <dhowells@redhat.com>
>     cc: Marc Dionne <marc.dionne@auristor.com>
>     cc: Eric Dumazet <edumazet@google.com>
>     cc: "David S. Miller" <davem@davemloft.net>
>     cc: Jakub Kicinski <kuba@kernel.org>
>     cc: Paolo Abeni <pabeni@redhat.com>
>     cc: Simon Horman <horms@kernel.org>
>     cc: linux-afs@lists.infradead.org
>     cc: netdev@vger.kernel.org
>     cc: stable@kernel.org
>
> diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
> index 918f41d97a2f..8d874ea428ff 100644
> --- a/net/rxrpc/call_object.c
> +++ b/net/rxrpc/call_object.c
> @@ -694,6 +694,7 @@ static void rxrpc_destroy_call(struct work_struct *work)
>         rxrpc_put_bundle(call->bundle, rxrpc_bundle_put_call);
>         rxrpc_put_peer(call->peer, rxrpc_peer_put_call);
>         rxrpc_put_local(call->local, rxrpc_local_put_call);
> +       key_put(call->key);
>         call_rcu(&call->rcu, rxrpc_rcu_free_call);
>  }
>
>


--
Anderson Nascimento
Allele Security Intelligence
https://www.allelesecurity.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  2026-03-13 13:23 ` [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt() Anderson Nascimento
@ 2026-03-19 16:10   ` Simon Horman
  2026-03-19 16:55     ` David Howells
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2026-03-19 16:10 UTC (permalink / raw)
  To: Anderson Nascimento
  Cc: dhowells, marc.dionne, davem, edumazet, kuba, pabeni, linux-afs,
	netdev, linux-kernel

On Fri, Mar 13, 2026 at 10:23:26AM -0300, Anderson Nascimento wrote:
> In rxrpc_setsockopt(), the code checks 'rx->key' when handling the
> RXRPC_SECURITY_KEYRING option. However, this appears to be a logic error.
> The code should be checking 'rx->securities' to determine if a keyring
> has already been defined for the socket.
> 
> Currently, if a user calls setsockopt(RXRPC_SECURITY_KEYRING) multiple
> times on the same socket, the check 'if (rx->key)' fails to block
> subsequent calls because 'rx->key' has not been defined by the function.
> This results in a reference count leak on the keyring.
> 
> This patch changes the check to 'rx->securities' to correctly identify
> if the socket security keyring has already been configured, returning -EINVAL
> on subsequent attempts.
> 
> Before the patch:
> 
> It shows the keyring reference counter elevated.
> 
> $ cat /proc/keys | grep AFSkeys1
> 27aca8ae I--Q--- 24469721 perm 3f010000  1000  1000 keyring   AFSkeys1: empty
> $
> 
> After the patch:
> 
> The keyring reference counter remains stable and subsequent calls return an error:
> 
> $ ./poc
> setsockopt: Invalid argument
> $
> 
> Fixes: 17926a7 ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")

nit: Please use 12 (or more if needed to avoid a collision) bytes
     for the hash in Fixes tags.

> Signed-off-by: Anderson Nascimento <anderson@allelesecurity.com>

...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  2026-03-19 16:10   ` Simon Horman
@ 2026-03-19 16:55     ` David Howells
  2026-03-20  8:24       ` Simon Horman
  0 siblings, 1 reply; 10+ messages in thread
From: David Howells @ 2026-03-19 16:55 UTC (permalink / raw)
  To: Simon Horman
  Cc: dhowells, Anderson Nascimento, marc.dionne, davem, edumazet, kuba,
	pabeni, linux-afs, netdev, linux-kernel

Simon Horman <horms@kernel.org> wrote:

> > Fixes: 17926a7 ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
> 
> nit: Please use 12 (or more if needed to avoid a collision) bytes
>      for the hash in Fixes tags.

I fixed this when I posted my collection of rxrpc patches, including this one.

David


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt()
  2026-03-19 16:55     ` David Howells
@ 2026-03-20  8:24       ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2026-03-20  8:24 UTC (permalink / raw)
  To: David Howells
  Cc: Anderson Nascimento, marc.dionne, davem, edumazet, kuba, pabeni,
	linux-afs, netdev, linux-kernel

On Thu, Mar 19, 2026 at 04:55:53PM +0000, David Howells wrote:
> Simon Horman <horms@kernel.org> wrote:
> 
> > > Fixes: 17926a7 ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
> > 
> > nit: Please use 12 (or more if needed to avoid a collision) bytes
> >      for the hash in Fixes tags.
> 
> I fixed this when I posted my collection of rxrpc patches, including this one.

Thanks David,

Sorry for not noticing that.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-20  8:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 13:23 [PATCH 0/2] rxrpc: Fix key and keyring reference count leaks Anderson Nascimento
2026-03-13 13:23 ` [PATCH 1/2] rxrpc: Fix keyring reference count leak in rxrpc_setsockopt() Anderson Nascimento
2026-03-19 16:10   ` Simon Horman
2026-03-19 16:55     ` David Howells
2026-03-20  8:24       ` Simon Horman
2026-03-13 13:23 ` [PATCH 2/2] rxrpc: Fix key reference count leak in rxrpc_alloc_client_call() Anderson Nascimento
2026-03-18 21:46   ` David Howells
2026-03-18 22:20   ` David Howells
2026-03-18 22:30     ` Anderson Nascimento
2026-03-19 14:46     ` Anderson Nascimento

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox