netdev - Re: [PATCH] decnet: fix possible NULL deref in dnet_select

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: <20140407.151854.411047851818388937.davem@davemloft.net>
Date:	Mon, 07 Apr 2014 15:18:54 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	sasha.levin@...cle.com, netdev@...r.kernel.org,
	linux-decnet-user@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, davej@...hat.com
Subject: Re: [PATCH] decnet: fix possible NULL deref in dnet_select_source()

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Sun, 06 Apr 2014 14:59:14 -0700

> From: Eric Dumazet <edumazet@...gle.com>
> 
> dnet_select_source() should make sure dn_ptr is not NULL.
> 
> While looking at this decnet code, I believe I found a device
> reference leak, lets fix it as well.
>  
> Reported-by: Sasha Levin <sasha.levin@...cle.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
> It seems this bug is very old, no recent change is involved.

The callers work hard to ensure this.

Analyzing all call sites:

1) __dn_fib_res_prefsrc() uses the FIB entry device pointer, we should not
   be adding FIB entries pointing to devices which do not have their
   decnet private initialized yet.

2) dn_route_output_slow()

   The paths leading to the dnet_select_address() call(s) check if
   dev_out->dn_ptr is not NULL, except when using loopback.

In some other paths the device comes from neigh->dev, from which the
'neigh' was looked up in dn_neigh_table.  There should not be neighbour
entries in this table pointing to devices which do not have their
decnet private setup yet.

And in the loopback case, it is the decnet stack's responsibility to
make sure ->dn_ptr is setup properly, else it should fail the module
load and stack initialization.

I think there is some core fundamental issue here, and just adding
a NULL check to dnet_select_source() is just papering around the issue.

Please look closer at the stack trace, this code, and my analysis
above to figure out what's really going on so we can fix this properly.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html