netdev - Re: [PATCH net] net: neighbour: add neighbour dead check for neigh_timer

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131218154648.GD3505@order.stressinduktion.org>
Date:	Wed, 18 Dec 2013 16:46:48 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Ding Tianhong <dthxman@...il.com>
Cc:	Ding Tianhong <dingtianhong@...wei.com>,
	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>, yoshfuji@...ux-ipv6.org,
	joe@...ches.com, vfalico@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: neighbour: add neighbour dead check for neigh_timer_handler()

On Wed, Dec 18, 2013 at 11:12:51PM +0800, Ding Tianhong wrote:
> 于 2013/12/18 22:27, Hannes Frederic Sowa 写道:
> > On Wed, Dec 18, 2013 at 07:57:40PM +0800, Ding Tianhong wrote:
> >> yes, I cannot repruduce the bug again.
> > 
> > Hmm, it actually seems hard to hit even if the race happens. Even if slab
> > poisoning is active it would only hit if ->solicit would be called again,
> > because that is the only pointer dereference directly used in the old memory.
> > 
> > neigh_alloc allocates memory with kzalloc, so it would null out that memory,
> > so the race would not only have to race with kfree, the memory needs to be
> > reallocated in the mean time.
> > 
> > I would suggest adding some poisoning manually in neigh_release before kfree
> > and check for this in all periodic called functions. Maybe we can see it
> > again?
> >
> Great, thanks for your help, I think make the neigh_release not kfree neighbour until
> the timer is over is a clear way to  fix this, maybe you could another idea, glad to
> hear your opinion.

But I don't suggest this as an fix, just as a help for debugging this issue.

Maybe you could also store the _RET_IP_ in the to be freed struct neighbour
(just before kfree) and thus have it available in case the machine panics (or
simply print it with printk).

Maybe it would make sense to use kmem_cache_create and kmem_cache_alloc for
struct neighs so we can better utilize the slub debugging features.

Greetings,

  Hannes

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