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

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: <CAM_iQpV20zj=ZcQQy8TckQr2CJgvj=YDxgOsXnHj4pACOiD--g@mail.gmail.com>
Date:   Mon, 4 Dec 2017 11:23:13 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        tipc-discussion@...ts.sourceforge.net,
        Jon Maloy <jon.maloy@...csson.com>,
        Ying Xue <ying.xue@...driver.com>
Subject: Re: [Patch net v2] tipc: fix a null pointer deref on error path

On Mon, Dec 4, 2017 at 10:57 AM, David Miller <davem@...emloft.net> wrote:
> From: Cong Wang <xiyou.wangcong@...il.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@...glegroups.com>
>> Cc: Jon Maloy <jon.maloy@...csson.com>
>> Cc: Ying Xue <ying.xue@...driver.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@...il.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?