[PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

Subsystems: networking [general], rxrpc sockets (af_rxrpc), the rest

STALE1727d

9 messages, 4 authors, 2021-11-29 · open the first message on its own page

[PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: David Howells <dhowells@redhat.com>
Date: 2021-11-24 14:37:52

From: Eiichi Tsukata <redacted>

Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
holds a ref to rxrpc_peer.

[DH: v2: Changed to abstract out the bundle freeing code into a function]

Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
Signed-off-by: Eiichi Tsukata <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/r/20211121041608.133740-1-eiichi.tsukata@nutanix.com/ # v1
---

 net/rxrpc/conn_client.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index dbea0bfee48e..8120138dac01 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -135,16 +135,20 @@ struct rxrpc_bundle *rxrpc_get_bundle(struct rxrpc_bundle *bundle)
 	return bundle;
 }
 
+static void rxrpc_free_bundle(struct rxrpc_bundle *bundle)
+{
+	rxrpc_put_peer(bundle->params.peer);
+	kfree(bundle);
+}
+
 void rxrpc_put_bundle(struct rxrpc_bundle *bundle)
 {
 	unsigned int d = bundle->debug_id;
 	unsigned int u = atomic_dec_return(&bundle->usage);
 
 	_debug("PUT B=%x %u", d, u);
-	if (u == 0) {
-		rxrpc_put_peer(bundle->params.peer);
-		kfree(bundle);
-	}
+	if (u == 0)
+		rxrpc_free_bundle(bundle);
 }
 
 /*
@@ -328,7 +332,7 @@ static struct rxrpc_bundle *rxrpc_look_up_bundle(struct rxrpc_conn_parameters *c
 	return candidate;
 
 found_bundle_free:
-	kfree(candidate);
+	rxrpc_free_bundle(candidate);
 found_bundle:
 	rxrpc_get_bundle(bundle);
 	spin_unlock(&local->client_bundles_lock);

[PATCH 2/2] rxrpc: Fix rxrpc_local leak in rxrpc_lookup_peer()

From: David Howells <dhowells@redhat.com>
Date: 2021-11-24 14:38:06

From: Eiichi Tsukata <redacted>

Need to call rxrpc_put_local() for peer candidate before kfree() as it
holds a ref to rxrpc_local.

[DH: v2: Changed to abstract the peer freeing code out into a function]

Fixes: 9ebeddef58c4 ("rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record")
Signed-off-by: Eiichi Tsukata <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/all/20211121041608.133740-2-eiichi.tsukata@nutanix.com/ # v1
---

 net/rxrpc/peer_object.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 68396d052052..0298fe2ad6d3 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -299,6 +299,12 @@ static struct rxrpc_peer *rxrpc_create_peer(struct rxrpc_sock *rx,
 	return peer;
 }
 
+static void rxrpc_free_peer(struct rxrpc_peer *peer)
+{
+	rxrpc_put_local(peer->local);
+	kfree_rcu(peer, rcu);
+}
+
 /*
  * Set up a new incoming peer.  There shouldn't be any other matching peers
  * since we've already done a search in the list from the non-reentrant context
@@ -365,7 +371,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_sock *rx,
 		spin_unlock_bh(&rxnet->peer_hash_lock);
 
 		if (peer)
-			kfree(candidate);
+			rxrpc_free_peer(candidate);
 		else
 			peer = candidate;
 	}
@@ -420,8 +426,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
 	list_del_init(&peer->keepalive_link);
 	spin_unlock_bh(&rxnet->peer_hash_lock);
 
-	rxrpc_put_local(peer->local);
-	kfree_rcu(peer, rcu);
+	rxrpc_free_peer(peer);
 }
 
 /*
@@ -457,8 +462,7 @@ void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
 	if (n == 0) {
 		hash_del_rcu(&peer->hash_link);
 		list_del_init(&peer->keepalive_link);
-		rxrpc_put_local(peer->local);
-		kfree_rcu(peer, rcu);
+		rxrpc_free_peer(peer);
 	}
 }
 

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-11-26 03:29:36

On Wed, 24 Nov 2021 14:37:33 +0000 David Howells wrote:
From: Eiichi Tsukata <redacted>

Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
holds a ref to rxrpc_peer.

[DH: v2: Changed to abstract out the bundle freeing code into a function]

Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
Signed-off-by: Eiichi Tsukata <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/r/20211121041608.133740-1-eiichi.tsukata@nutanix.com/ # v1
Are these supposed to go to net? They are addressed To: the author.

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: David Howells <dhowells@redhat.com>
Date: 2021-11-26 08:08:09

Jakub Kicinski [off-list ref] wrote:
Are these supposed to go to net? They are addressed To: the author.
I'm hoping the author rechecks/reviews them.  I commented on his original
submission that I thought they could be done slightly differently.

David

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: Eiichi Tsukata <hidden>
Date: 2021-11-26 08:36:58

On Nov 26, 2021, at 17:05, David Howells [off-list ref] wrote:

Jakub Kicinski [off-list ref] wrote:
quoted
Are these supposed to go to net? They are addressed To: the author.
I'm hoping the author rechecks/reviews them.  I commented on his original
submission that I thought they could be done slightly differently.
Thanks, I’ve tested them with my environment. Looks good.

Eiichi

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: Marc Dionne <marc.dionne@auristor.com>
Date: 2021-11-26 13:15:15

On Wed, Nov 24, 2021 at 10:37 AM David Howells [off-list ref] wrote:
quoted hunk
From: Eiichi Tsukata <redacted>

Need to call rxrpc_put_peer() for bundle candidate before kfree() as it
holds a ref to rxrpc_peer.

[DH: v2: Changed to abstract out the bundle freeing code into a function]

Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager")
Signed-off-by: Eiichi Tsukata <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/r/20211121041608.133740-1-eiichi.tsukata@nutanix.com/ # v1
---

 net/rxrpc/conn_client.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index dbea0bfee48e..8120138dac01 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -135,16 +135,20 @@ struct rxrpc_bundle *rxrpc_get_bundle(struct rxrpc_bundle *bundle)
        return bundle;
 }

+static void rxrpc_free_bundle(struct rxrpc_bundle *bundle)
+{
+       rxrpc_put_peer(bundle->params.peer);
+       kfree(bundle);
+}
+
 void rxrpc_put_bundle(struct rxrpc_bundle *bundle)
 {
        unsigned int d = bundle->debug_id;
        unsigned int u = atomic_dec_return(&bundle->usage);

        _debug("PUT B=%x %u", d, u);
-       if (u == 0) {
-               rxrpc_put_peer(bundle->params.peer);
-               kfree(bundle);
-       }
+       if (u == 0)
+               rxrpc_free_bundle(bundle);
 }

 /*
@@ -328,7 +332,7 @@ static struct rxrpc_bundle *rxrpc_look_up_bundle(struct rxrpc_conn_parameters *c
        return candidate;

 found_bundle_free:
-       kfree(candidate);
+       rxrpc_free_bundle(candidate);
 found_bundle:
        rxrpc_get_bundle(bundle);
        spin_unlock(&local->client_bundles_lock);
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>

Marc

Re: [PATCH 2/2] rxrpc: Fix rxrpc_local leak in rxrpc_lookup_peer()

From: Marc Dionne <marc.dionne@auristor.com>
Date: 2021-11-26 13:17:59

On Wed, Nov 24, 2021 at 10:38 AM David Howells [off-list ref] wrote:
quoted hunk
From: Eiichi Tsukata <redacted>

Need to call rxrpc_put_local() for peer candidate before kfree() as it
holds a ref to rxrpc_local.

[DH: v2: Changed to abstract the peer freeing code out into a function]

Fixes: 9ebeddef58c4 ("rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record")
Signed-off-by: Eiichi Tsukata <redacted>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Link: https://lore.kernel.org/all/20211121041608.133740-2-eiichi.tsukata@nutanix.com/ # v1
---

 net/rxrpc/peer_object.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 68396d052052..0298fe2ad6d3 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -299,6 +299,12 @@ static struct rxrpc_peer *rxrpc_create_peer(struct rxrpc_sock *rx,
        return peer;
 }

+static void rxrpc_free_peer(struct rxrpc_peer *peer)
+{
+       rxrpc_put_local(peer->local);
+       kfree_rcu(peer, rcu);
+}
+
 /*
  * Set up a new incoming peer.  There shouldn't be any other matching peers
  * since we've already done a search in the list from the non-reentrant context
@@ -365,7 +371,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_sock *rx,
                spin_unlock_bh(&rxnet->peer_hash_lock);

                if (peer)
-                       kfree(candidate);
+                       rxrpc_free_peer(candidate);
                else
                        peer = candidate;
        }
@@ -420,8 +426,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
        list_del_init(&peer->keepalive_link);
        spin_unlock_bh(&rxnet->peer_hash_lock);

-       rxrpc_put_local(peer->local);
-       kfree_rcu(peer, rcu);
+       rxrpc_free_peer(peer);
 }

 /*
@@ -457,8 +462,7 @@ void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
        if (n == 0) {
                hash_del_rcu(&peer->hash_link);
                list_del_init(&peer->keepalive_link);
-               rxrpc_put_local(peer->local);
-               kfree_rcu(peer, rcu);
+               rxrpc_free_peer(peer);
        }
 }
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>

Marc

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: David Howells <dhowells@redhat.com>
Date: 2021-11-29 15:42:35

Eiichi Tsukata [off-list ref] wrote:
Thanks, I’ve tested them with my environment. Looks good.
Thanks.

David

Re: [PATCH 1/2] rxrpc: Fix rxrpc_peer leak in rxrpc_look_up_bundle()

From: David Howells <dhowells@redhat.com>
Date: 2021-11-29 15:56:10

Hi Jakub,
Are these supposed to go to net? They are addressed To: the author.
I've posted a new set to netdev that has the Acks from Marc added for you to
pick up.

David
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help