netdev - Re: [PATCH net] af_unix: Guard against other == sk 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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a8n5able.fsf@doppelsaurus.mobileactivedefense.com>
Date:	Fri, 12 Feb 2016 20:59:57 +0000
From:	Rainer Weikusat <rweikusat@...ileactivedefense.com>
To:	Ben Hutchings <ben@...adent.org.uk>
Cc:	Rainer Weikusat <rweikusat@...ileactivedefense.com>,
	Philipp Hahn <pmhahn@...ahn.de>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	Sasha Levin <sasha.levin@...cle.com>,
	"David S. Miller" <davem@...emloft.net>,
	linux-kernel@...r.kernel.org, Karolin Seeger <kseeger@...ba.org>,
	Jason Baron <jbaron@...mai.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Arvid Requate <requate@...vention.de>,
	Stefan Gohmann <gohmann@...vention.de>, netdev@...r.kernel.org
Subject: Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

Ben Hutchings <ben@...adent.org.uk> writes:
> On Fri, 2016-02-12 at 20:17 +0000, Rainer Weikusat wrote:

[...]

>>>> I don't think this should apply when
>>>> receiving and sending sockets are identical. But that's just my
>>>> opinion. The other option would be to avoid the unix_state_double_lock
>>>> for sk == other.
>>> 
>>> Given that unix_state_double_lock() already handles sk == other, I'm
>>> not sure why you think it needs to be avoided.
>> 
>> Because the whole complication of restarting the operation after locking
>> both sk and other because other had to be unlocked before calling
>> unix_state_double_lock is useless for this case:

[...]

> Well of course it's useless, but it's also harmless.  

As is adding a

for (i = 0; i < 1000000; ++i);

between any two statements. And this isn't even entirely true as the
pointless double-lock will then require "did we pointlessly
doube-lock" checks elsewhere. I think it should be possible to do this
in a simpler way by not pointlessly double-locking (this may be
wrong but it's worth a try).

> If we really wanted to optimise this we could also skip unlocking if
> other < sk.

I wouldn't want to hardcode assumptions about the unix_state_double_lock
algorithm in functions using it.