RE: [Patch net v2] tipc: fix a null pointer deref on error path

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-
owner@vger.kernel.org] On Behalf Of David Miller
Sent: Monday, December 04, 2017 13:57
To: xiyou.wangcong@gmail.com
Cc: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; Jon
Maloy [off-list ref]; Ying Xue [off-list ref]
Subject: Re: [Patch net v2] tipc: fix a null pointer deref on error path

From: Cong Wang <redacted>
Date: Mon,  4 Dec 2017 10:31:43 -0800

quoted

In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails we call
tipc_close_conn() to clean up, but in this case calling conn_put() is
just enough.

This fixes the folllowing crash:

 ...

quoted

Fixes: 14c04493cb77 ("tipc: add ability to order and receive topology
events in driver")
Reported-by: syzbot <redacted>
Cc: Jon Maloy <redacted>
Cc: Ying Xue <redacted>
Signed-off-by: Cong Wang <redacted>

 ...

quoted

@@ -511,7 +511,7 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32

port, u32 type,

quoted

 	s = con->server;
 	scbr = s->tipc_conn_new(*conid);
 	if (!scbr) {
-		tipc_close_conn(con);
+		conn_put(con);
 		return false;
 	}

--
2.13.0

It looks like tipc_accept_from_sock() has a similar problem?  The
tipc_close_conn() will get invoked indirectly from the sock_release()
path right?

No, it doesn't. There will be a 'leaked' conn instance which will remain in the reference table until it is flushed during module removal.
We'll fix this in a separate patch. Cong's fix is correct.

///jon