linux-kernel - Re: [PATCH] unix: avoid use-after-free in ep_remove_wait

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]

Message-ID: <87si3w9z3m.fsf@doppelsaurus.mobileactivedefense.com>
Date:	Mon, 23 Nov 2015 21:37:17 +0000
From:	Rainer Weikusat <rweikusat@...ileactivedefense.com>
To:	David Miller <davem@...emloft.net>
Cc:	rweikusat@...ileactivedefense.com, jbaron@...mai.com,
	dvyukov@...gle.com, syzkaller@...glegroups.com, mkubecek@...e.cz,
	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, hannes@...essinduktion.org,
	dhowells@...hat.com, paul@...l-moore.com, salyzyn@...roid.com,
	sds@...ho.nsa.gov, ying.xue@...driver.com, netdev@...r.kernel.org,
	kcc@...gle.com, glider@...gle.com, andreyknvl@...gle.com,
	sasha.levin@...cle.com, jln@...gle.com, keescook@...gle.com,
	minipli@...glemail.com
Subject: Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

David Miller <davem@...emloft.net> writes:
> From: Rainer Weikusat <rweikusat@...ileactivedefense.com>
>> Rainer Weikusat <rweikusat@...ileactivedefense.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/