Re: [PATCH] tipc: Fix potential tipc_node refcnt leak in tipc_rcv

Re: [PATCH] tipc: Fix potential tipc_node refcnt leak in tipc_rcv

[Markus Elfring]

> The issue happens in some paths of tipc_rcv(), which forget to decrease
> the refcnt increased by tipc_node_find() and will cause a refcnt leak.

How do you think about to mention the term “reference counting” in
the commit message?

Would you like to add the tag “Fixes” to the change description?

Regards,
Markus

Re: [PATCH] tipc: Fix potential tipc_node refcnt leak in tipc_rcv

[David Miller]

From: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Date: Wed, 15 Apr 2020 16:40:28 +0800

> tipc_rcv() invokes tipc_node_find() twice, which returns a reference of
> the specified tipc_node object to "n" with increased refcnt.
> 
> When tipc_rcv() returns or a new object is assigned to "n", the original
> local reference of "n" becomes invalid, so the refcount should be
> decreased to keep refcount balanced.
> 
> The issue happens in some paths of tipc_rcv(), which forget to decrease
> the refcnt increased by tipc_node_find() and will cause a refcnt leak.
> 
> Fix this issue by calling tipc_node_put() before the original object
> pointed by "n" becomes invalid.
> 
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>

Applied and queued up for -stable.

[PATCH] tipc: Fix potential tipc_node refcnt leak in tipc_rcv

[Xiyu Yang]

tipc_rcv() invokes tipc_node_find() twice, which returns a reference of
the specified tipc_node object to "n" with increased refcnt.

When tipc_rcv() returns or a new object is assigned to "n", the original
local reference of "n" becomes invalid, so the refcount should be
decreased to keep refcount balanced.

The issue happens in some paths of tipc_rcv(), which forget to decrease
the refcnt increased by tipc_node_find() and will cause a refcnt leak.

Fix this issue by calling tipc_node_put() before the original object
pointed by "n" becomes invalid.

Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
 net/tipc/node.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 0c88778c88b5..d50be9a3d479 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2037,6 +2037,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
 		n = tipc_node_find_by_id(net, ehdr->id);
 	}
 	tipc_crypto_rcv(net, (n) ? n->crypto_rx : NULL, &skb, b);
+	tipc_node_put(n);
 	if (!skb)
 		return;
 
@@ -2089,7 +2090,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
 	/* Check/update node state before receiving */
 	if (unlikely(skb)) {
 		if (unlikely(skb_linearize(skb)))
-			goto discard;
+			goto out_node_put;
 		tipc_node_write_lock(n);
 		if (tipc_node_check_state(n, skb, bearer_id, &xmitq)) {
 			if (le->link) {
@@ -2118,6 +2119,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
 	if (!skb_queue_empty(&xmitq))
 		tipc_bearer_xmit(net, bearer_id, &xmitq, &le->maddr, n);
 
+out_node_put:
 	tipc_node_put(n);
 discard:
 	kfree_skb(skb);
-- 
2.7.4