From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752435AbbJKNcO (ORCPT ); Sun, 11 Oct 2015 09:32:14 -0400 Received: from tiger.mobileactivedefense.com ([217.174.251.109]:38502 "EHLO tiger.mobileactivedefense.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751973AbbJKNcM (ORCPT ); Sun, 11 Oct 2015 09:32:12 -0400 From: Rainer Weikusat To: Hannes Frederic Sowa Cc: Jason Baron , davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, minipli@googlemail.com, normalperson@yhbt.net, eric.dumazet@gmail.com, rweikusat@mobileactivedefense.com, viro@zeniv.linux.org.uk, davidel@xmailserver.org, dave@stgolabs.net, olivier@mauras.ch, pageexec@freemail.hu, torvalds@linux-foundation.org, peterz@infradead.org, joe@perches.com Subject: Re: [PATCH v4 1/3] net: unix: fix use-after-free in unix_dgram_poll() In-Reply-To: <87io6gayze.fsf@stressinduktion.org> (Hannes Frederic Sowa's message of "Fri, 09 Oct 2015 16:38:29 +0200") References: <8f8dfc6fdb8015091b58509044b15489df261461.1444363559.git.jbaron@akamai.com> <87io6gayze.fsf@stressinduktion.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux) Date: Sun, 11 Oct 2015 14:30:58 +0100 Message-ID: <87r3l1cz1p.fsf@doppelsaurus.mobileactivedefense.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (tiger.mobileactivedefense.com [217.174.251.109]); Sun, 11 Oct 2015 14:31:07 +0100 (BST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hannes Frederic Sowa writes: > Jason Baron writes: > >> The unix_dgram_poll() routine calls sock_poll_wait() not only for the wait >> queue associated with the socket s that we are poll'ing against, but also calls >> sock_poll_wait() for a remote peer socket p, if it is connected. Thus, >> if we call poll()/select()/epoll() for the socket s, there are then >> a couple of code paths in which the remote peer socket p and its associated >> peer_wait queue can be freed before poll()/select()/epoll() have a chance >> to remove themselves from the remote peer socket. >> >> The way that remote peer socket can be freed are: >> >> 1. If s calls connect() to a connect to a new socket other than p, it will >> drop its reference on p, and thus a close() on p will free it. >> >> 2. If we call close on p(), then a subsequent sendmsg() from s, will drop >> the final reference to p, allowing it to be freed. >> >> Address this issue, by reverting unix_dgram_poll() to only register with >> the wait queue associated with s and register a callback with the remote peer >> socket on connect() that will wake up the wait queue associated with s. If >> scenarios 1 or 2 occur above we then simply remove the callback from the >> remote peer. This then presents the expected semantics to poll()/select()/ >> epoll(). >> >> I've implemented this for sock-type, SOCK_RAW, SOCK_DGRAM, and SOCK_SEQPACKET >> but not for SOCK_STREAM, since SOCK_STREAM does not use unix_dgram_poll(). >> >> Introduced in commit ec0d215f9420 ("af_unix: fix 'poll for write'/connected >> DGRAM sockets"). >> >> Tested-by: Mathias Krause >> Signed-off-by: Jason Baron > > While I think this approach works, I haven't seen where the current code > leaks a reference. It doesn't "leak a reference" (strictly). It possibly registers a wait queue with whatever invoked the poll-routine which belongs to the peer socket of the socket poll was called on. And the inherent problem with that is that the lifetime of the peer socket is not necessarily the same as the lifetime of the polled socket. If the polled socket is disconnected from its peer while still being polled (or registered for being polled), the former peer may be freed despite the polling code (of whatever provenience) still references the peer_wait member of the unix socket structure for this socket. As pointed out in the original mail, two ways for this to happen is to call connect on the polled socket or cause a unix_dgram_sendmsg call on that after the peer socket was closed.