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

From: Cong Wang <redacted>
Date: Mon, 4 Dec 2017 11:23:13 -0800

On Mon, Dec 4, 2017 at 10:57 AM, David Miller [off-list ref] wrote:

quoted

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,
      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?

Not sure, the sock_release() in tipc_accept_from_sock() is for
kernel_accept(), not for tipc_alloc_conn(). Or maybe it is hiding
deep in the call chain that I miss?

I guess I'm trying to figure out where 'newcon' is released when the
call to s->tipc_conn_new() on it fails.

It looks similar to the situation you are fixing here, which is why I
am mentioning it.