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: <AM4PR07MB171452785DE3ADC8B08388219A3D0@AM4PR07MB1714.eurprd07.prod.outlook.com>
Date:   Tue, 5 Dec 2017 01:56:48 +0000
From:   Jon Maloy <jon.maloy@...csson.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
CC:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "tipc-discussion@...ts.sourceforge.net" 
        <tipc-discussion@...ts.sourceforge.net>,
        Ying Xue <ying.xue@...driver.com>
Subject: RE: [Patch net v2] tipc: fix a null pointer deref on error path



> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@...il.com]
> Sent: Monday, December 04, 2017 17:34
> To: Jon Maloy <jon.maloy@...csson.com>
> Cc: David Miller <davem@...emloft.net>; Linux Kernel Network Developers
> <netdev@...r.kernel.org>; tipc-discussion@...ts.sourceforge.net; 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 12:32 PM, Jon Maloy <jon.maloy@...csson.com>
> wrote:
> >
> >> You are right. The right solution is to just call conn_put() twice here.
> >> I already have a patch ready for this, but it is part of a series
> >> that needs more review.
> >> I should probably post it separately...
> >
> > Well, calling conn_put() twice was ok in my series, but in the current
> upstream version it is not enough.
> > I will find a different short term solution.
> >
> 
> IMHO, for tipc_topsrv_kern_subscr() my v2 patch is more correct, and for
> tipc_accept_from_sock(), v1 is needed too.

Absolutely. I already acked v2, but maybe I wasn't clear enough about that.
What I was referring to above was tipc_kern_unsubscr(), where you also, correctly, identified a problem.
That problem is reduced, but not totally eliminated by your v1, so I am ok with that one too for now.
We have a fully safe solution for this in the pipe.

> 
> Of course v1 could fix both, but still v2 is better than v1 if we only consider
> tipc_topsrv_kern_subscr() case. So this depends if we want to consider
> these 2 paths as 2 cases or 1 case. _I think_ it is better to consider them
> separately since we already v2 is best fix for tipc_topsrv_kern_subscr(),
> while still not sure what is the best for tipc_accept_from_sock().

The tipc_accept_from_sock() problem is as far as I can see totally analogous to the tipc_kern_subscr() one, and requires the same solution. This is what my patch is providing.

BR
///jon