netdev - Re: [RFC] unix: fix use-after-free in unix_dgram

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]

Date:	Thu, 29 Oct 2015 14:23:50 +0000
From:	Rainer Weikusat <rweikusat@...ileactivedefense.com>
To:	Jason Baron <jbaron@...mai.com>
Cc:	Rainer Weikusat <rweikusat@...ileactivedefense.com>,
	davem@...emloft.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, minipli@...glemail.com,
	normalperson@...t.net, eric.dumazet@...il.com,
	viro@...iv.linux.org.uk, davidel@...ilserver.org,
	dave@...olabs.net, olivier@...ras.ch, pageexec@...email.hu,
	torvalds@...ux-foundation.org, peterz@...radead.org
Subject: Re: [RFC] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jbaron@...mai.com> writes:
> On 10/28/2015 12:46 PM, Rainer Weikusat wrote:
>> Rainer Weikusat <rw@...pelsaurus.mobileactivedefense.com> writes:
>>> Jason Baron <jbaron@...mai.com> writes:

[...]

>> and the not-so-nice additional property that the connect and
>> disconnect functions need to take the peer_wait.lock spinlock
>> explicitly so that this lock is used to ensure that no two threads
>> modifiy the private pointer of the client wait_queue_t.
>
> Hmmm...I thought these were already all guarded by unix_state_lock(sk).
> In any case, rest of the patch overall looks good to me.

All but the one in unix_dgram_peer_wake_relay: That's
called with the wait queue lock of the peer_wait queue held and even if
it was desirable to acquire the state lock of the involved socket, it
wouldn't easily be possible because 'other code' (in poll and sendmsg)
acquires the wait queue lock while holding the state lock.

The idea behind this is that the state lock ensures that the 'connection
state' doesn't change while some code is examining it and that the wait
queue lock of the peer_wait queue is (slightly ab-)used to ensure
exclusive access to the private pointer which guards against concurrent
inserts or removes of the same wait_queue_t.

Poking around in the implementation of abstract interfaces like this
isn't something I'm overly fond of but 'other code' does this too, eg,
in eventfd.c

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