From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from tiger.mobileactivedefense.com ([217.174.251.109]:49046 "EHLO
	tiger.mobileactivedefense.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1754398AbbKWVh6 (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 23 Nov 2015 16:37:58 -0500
From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
To: David Miller <davem@davemloft.net>
Cc: rweikusat@mobileactivedefense.com, jbaron@akamai.com,
	dvyukov@google.com, syzkaller@googlegroups.com, mkubecek@suse.cz,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, hannes@stressinduktion.org,
	dhowells@redhat.com, paul@paul-moore.com, salyzyn@android.com,
	sds@tycho.nsa.gov, ying.xue@windriver.com, netdev@vger.kernel.org,
	kcc@google.com, glider@google.com, andreyknvl@google.com,
	sasha.levin@oracle.com, jln@google.com, keescook@google.com,
	minipli@googlemail.com
Subject: Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue
In-Reply-To: <20151123.123033.522701221384821229.davem@davemloft.net> (David
	Miller's message of "Mon, 23 Nov 2015 12:30:33 -0500 (EST)")
References: <87ziydvasn.fsf_-_@doppelsaurus.mobileactivedefense.com>
	<87d1v57dif.fsf@doppelsaurus.mobileactivedefense.com>
	<87vb8w2ulg.fsf_-_@doppelsaurus.mobileactivedefense.com>
	<20151123.123033.522701221384821229.davem@davemloft.net>
Date: Mon, 23 Nov 2015 21:37:17 +0000
Message-ID: <87si3w9z3m.fsf@doppelsaurus.mobileactivedefense.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

David Miller <davem@davemloft.net> writes:
> From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
>> Rainer Weikusat <rweikusat@mobileactivedefense.com> writes:
>> An AF_UNIX datagram socket being the client in an n:1 association

[...]

> Applied and queued up for -stable,

I'm sorry for this 13th hour request/ suggestion but while thinking
about a reply to Dmitry, it occurred to me that the restart_locked/
sk_locked logic could be avoided by moving the test for this condition
in front of all the others while leaving the 'act on it' code at its
back, ie, reorganize unix_dgram_sendmsg such that it looks like this:

	unix_state_lock(other);
	
	if (unix_peer(other) != sk && unix_recvq_full(other)) {
		need_wait = 1;
		
		if (!timeo) {
			unix_state_unlock(other);
			unix_state_double_lock(sk, other);

			if (unix_peer(other) == sk ||
			    (unix_peer(sk) == other &&
			     !unix_dgram_peer_wake_me(sk, other)))
				need_wait = 0;

			unix_state_unlock(sk);
		}
	}

        /* original code here */
        
	if (need_wait) {
		if (timeo) {
			timeo = unix_wait_for_peer(other, timeo);

			err = sock_intr_errno(timeo);
			if (signal_pending(current))
				goto out_free;
			
			goto restart;
		}
		
		err = -EAGAIN;
		goto out_unlock;
	}

        /* original tail here */

This might cause a socket to be enqueued to the peer despite it's not
allowed to send to it but I don't think this matters much. This is a
less conservative modification but one which results in simpler code
overall. The kernel I'm currently running has been modified like this
and 'survived' the usual tests.