From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751180AbcBLVA1 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 12 Feb 2016 16:00:27 -0500
Received: from tiger.mobileactivedefense.com ([217.174.251.109]:49530 "EHLO
	tiger.mobileactivedefense.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1750926AbcBLVAZ convert rfc822-to-8bit
	(ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 12 Feb 2016 16:00:25 -0500
From: Rainer Weikusat <rweikusat@mobileactivedefense.com>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Rainer Weikusat <rweikusat@mobileactivedefense.com>,
        Philipp Hahn <pmhahn@pmhahn.de>,
        Hannes Frederic Sowa <hannes@stressinduktion.org>,
        Sasha Levin <sasha.levin@oracle.com>,
        "David S. Miller" <davem@davemloft.net>, linux-kernel@vger.kernel.org,
        Karolin Seeger <kseeger@samba.org>, Jason Baron <jbaron@akamai.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Arvid Requate <requate@univention.de>,
        Stefan Gohmann <gohmann@univention.de>, netdev@vger.kernel.org
Subject: Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg
In-Reply-To: <1455310037.2801.61.camel@decadent.org.uk> (Ben Hutchings's
	message of "Fri, 12 Feb 2016 20:47:17 +0000")
References: <56B4BF9D.9070609@pmhahn.de> <56BC90E7.7040007@pmhahn.de>
	<87fuwzkzr5.fsf@doppelsaurus.mobileactivedefense.com>
	<1455210224.2801.21.camel@decadent.org.uk>
	<87r3gjjgbu.fsf@doppelsaurus.mobileactivedefense.com>
	<87egcjcd5j.fsf@doppelsaurus.mobileactivedefense.com>
	<87r3gj11jc.fsf_-_@doppelsaurus.mobileactivedefense.com>
	<56BDA3A8.6070807@pmhahn.de>
	<8760xuvz5w.fsf@doppelsaurus.mobileactivedefense.com>
	<1455306847.2801.45.camel@decadent.org.uk>
	<87egchadk4.fsf@doppelsaurus.mobileactivedefense.com>
	<1455310037.2801.61.camel@decadent.org.uk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux)
Date: Fri, 12 Feb 2016 20:59:57 +0000
Message-ID: <87a8n5able.fsf@doppelsaurus.mobileactivedefense.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (tiger.mobileactivedefense.com [217.174.251.109]); Fri, 12 Feb 2016 21:00:05 +0000 (GMT)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Ben Hutchings <ben@decadent.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.