netdev - Re: [net 1/2] Revert "tipc: Fix potential tipc_node refcnt leak in tipc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200602053046.GA7054@example.com>
Date:   Tue, 2 Jun 2020 12:42:32 +0700
From:   "hoang.h.le" <hoang.h.le@...tech.com.au>
To:     Tuong Lien <tuong.t.lien@...tech.com.au>
Cc:     davem@...emloft.net, jmaloy@...hat.com, maloy@...jonn.com,
        ying.xue@...driver.com, netdev@...r.kernel.org,
        tipc-discussion@...ts.sourceforge.net
Subject: Re: [net 1/2] Revert "tipc: Fix potential tipc_node refcnt leak in
 tipc_rcv"

On Tue, Jun 02, 2020 at 11:46:40AM +0700, Tuong Lien wrote:
Tested-by: Hoang Le <hoang.h.le@...tech.com.au>

> This reverts commit de058420767df21e2b6b0f3bb36d1616fb962032.
> 
> There is no actual tipc_node refcnt leak as stated in the above commit.
> The refcnt is hold carefully for the case of an asynchronous decryption
> (i.e. -EINPROGRESS/-EBUSY and skb = NULL is returned), so that the node
> object cannot be freed in the meantime. The counter will be re-balanced
> when the operation's callback arrives with the decrypted buffer if any.
> In other cases, e.g. a synchronous crypto the counter will be decreased
> immediately when it is done.
> 
> Now with that commit, a kernel panic will occur when there is no node
> found (i.e. n = NULL) in the 'tipc_rcv()' or a premature release of the
> node object.
> 
> This commit solves the issues by reverting the said commit, but keeping
> one valid case that the 'skb_linearize()' is failed.
> 
> Acked-by: Jon Maloy <jmaloy@...hat.com>
> Signed-off-by: Tuong Lien <tuong.t.lien@...tech.com.au>
> ---
>  net/tipc/node.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 0312fb181d94..a4c2816c3746 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2038,7 +2038,6 @@ 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;
>