From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Miller <davem@davemloft.net>
Subject: Re: [Patch net v2] tipc: fix a null pointer deref on error path
Date: Mon, 04 Dec 2017 14:48:00 -0500 (EST)
Message-ID: <20171204.144800.504662839749640172.davem@davemloft.net>
References: <20171204183143.7395-1-xiyou.wangcong@gmail.com>
        <20171204.135716.731762293431821544.davem@davemloft.net>
        <CAM_iQpV20zj=ZcQQy8TckQr2CJgvj=YDxgOsXnHj4pACOiD--g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
        jon.maloy@ericsson.com, ying.xue@windriver.com
To: xiyou.wangcong@gmail.com
Return-path: <netdev-owner@vger.kernel.org>
Received: from shards.monkeyblade.net ([184.105.139.130]:52344 "EHLO
        shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751754AbdLDTsF (ORCPT
        <rfc822;netdev@vger.kernel.org>); Mon, 4 Dec 2017 14:48:05 -0500
In-Reply-To: <CAM_iQpV20zj=ZcQQy8TckQr2CJgvj=YDxgOsXnHj4pACOiD--g@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 4 Dec 2017 11:23:13 -0800

> On Mon, Dec 4, 2017 at 10:57 AM, David Miller <davem@davemloft.net> wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Mon,  4 Dec 2017 10:31:43 -0800
>>
>>> 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:
>>  ...
>>> Fixes: 14c04493cb77 ("tipc: add ability to order and receive topology events in driver")
>>> Reported-by: syzbot <syzkaller@googlegroups.com>
>>> Cc: Jon Maloy <jon.maloy@ericsson.com>
>>> Cc: Ying Xue <ying.xue@windriver.com>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>  ...
>>> @@ -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.