From mboxrd@z Thu Jan  1 00:00:00 1970
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1759936AbeAIQ7q (ORCPT <rfc822;ralf@linux-mips.org> + 1 other);
        Tue, 9 Jan 2018 11:59:46 -0500
Received: from shards.monkeyblade.net ([184.105.139.130]:34376 "EHLO
        shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753122AbeAIQ7p (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 9 Jan 2018 11:59:45 -0500
Date: Tue, 09 Jan 2018 11:59:43 -0500 (EST)
Message-Id: <20180109.115943.1711214677643324564.davem@davemloft.net>
To: nstange@suse.de
Cc: kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
        simo.ghannam@gmail.com, mkubecek@suse.cz, mbenes@suse.cz,
        sbrivio@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] net: ipv4: emulate READ_ONCE() on ->hdrincl
 bit-field in raw_sendmsg()
From: David Miller <davem@davemloft.net>
In-Reply-To: <20180108145444.19457-1-nstange@suse.de>
References: <20180103113700.049f7558@elisabeth>
        <20180108145444.19457-1-nstange@suse.de>
X-Mailer: Mew version 6.7 on Emacs 25.3 / Mule 6.0 (HANACHIRUSATO)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Tue, 09 Jan 2018 08:59:44 -0800 (PST)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Return-Path: <linux-kernel-owner@vger.kernel.org>

From: Nicolai Stange <nstange@suse.de>
Date: Mon,  8 Jan 2018 15:54:44 +0100

> Commit 8f659a03a0ba ("net: ipv4: fix for a race condition in
> raw_sendmsg") fixed the issue of possibly inconsistent ->hdrincl handling
> due to concurrent updates by reading this bit-field member into a local
> variable and using the thus stabilized value in subsequent tests.
> 
> However, aforementioned commit also adds the (correct) comment that
> 
>   /* hdrincl should be READ_ONCE(inet->hdrincl)
>    * but READ_ONCE() doesn't work with bit fields
>    */
> 
> because as it stands, the compiler is free to shortcut or even eliminate
> the local variable at its will.
> 
> Note that I have not seen anything like this happening in reality and thus,
> the concern is a theoretical one.
> 
> However, in order to be on the safe side, emulate a READ_ONCE() on the
> bit-field by doing it on the local 'hdrincl' variable itself:
> 
> 	int hdrincl = inet->hdrincl;
> 	hdrincl = READ_ONCE(hdrincl);
> 
> This breaks the chain in the sense that the compiler is not allowed
> to replace subsequent reads from hdrincl with reloads from inet->hdrincl.
> 
> Fixes: 8f659a03a0ba ("net: ipv4: fix for a race condition in raw_sendmsg")
> Signed-off-by: Nicolai Stange <nstange@suse.de>

Applied, thank you.