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]
Date:	Fri, 16 Jan 2015 16:48:30 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	subashab@...eaurora.org
Cc:	eric.dumazet@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH] net: ipv4: Fix incorrect free in ICMP receive

From: subashab@...eaurora.org
Date: Fri, 16 Jan 2015 20:59:14 -0000

>>skb_queue_purge() is also calling skb_orphan() on all skb
> From my reading, it looked like skb_queue_purge() is dequeuing and calling
> kfree_skb() which will release a reference. I did not see skb_orphan()
> being called directly. Am I missing something?
> I think that if it had really orphaned the skb, then this crash would not
> be seen in the first place.

The calls to skb->destructor(), done by skb_queue_purge() (via
kfree_skb()) do this.

But even if it didn't, the fact remains that we are operating on the
socket right here in the destructor.  It still exists and has not been
freed yet.

And furthermore, exactly what skb_orphan() does is call skb->destructor(),
_JUST_LIKE_ skb_queue_purge() will via kfree_skb().

So either sock_rfree() is safe to call here, or it isn't.  You are not
eliminating the calls to sock_rfree() which operate on this socket at
all.  If you did, then the socket memory counters would end up being
corrupts and the warnings would trigger:

	WARN_ON(atomic_read(&sk->sk_rmem_alloc));
	WARN_ON(atomic_read(&sk->sk_wmem_alloc));

You're just moving the skb->destructor() call up a few lines in the
same function, it makes therefore no sense why this would fix a bug
or not.
--
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

Powered by blists - more mailing lists