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 S1751973AbbJRAXV (ORCPT <rfc822;w@1wt.eu>);
	Sat, 17 Oct 2015 20:23:21 -0400
Received: from out4-smtp.messagingengine.com ([66.111.4.28]:47432 "EHLO
	out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1751560AbbJRAXT (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Sat, 17 Oct 2015 20:23:19 -0400
X-Sasl-enc: cLEKoJYAnED/hx0TtYX9WQ0nIO+Sf/GVsyTRpY8pU57c 1445127798
Date: Sat, 17 Oct 2015 17:23:17 -0700
From: Greg KH <greg@kroah.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Jaccon Bastiaansen <jaccon.bastiaansen@gmail.com>, x86@kernel.org,
        mingo@redhat.com, "H. Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        h.zuidam@computer.org, stable@vger.kernel.org
Subject: Re: [RFC]: Possible race condition in kernel futex code
Message-ID: <20151018002317.GH18971@kroah.com>
References: <CAGzjT4ez+gWr3BFQsEr-wma+vs6UZNJ+mRARx_BWoAKEJSsN=w@mail.gmail.com>
 <alpine.DEB.2.11.1510090946200.6097@nanos>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.11.1510090946200.6097@nanos>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Oct 09, 2015 at 10:06:41AM +0100, Thomas Gleixner wrote:
> On Mon, 5 Oct 2015, Jaccon Bastiaansen wrote:
> > We did some tests with different compilers, kernel versions and kernel
> > configs, with the following results:
> > 
> > Linux 3.12.48, x86_64_defconfig, GCC 4.6.1 :
> > copy_user_generic_unrolled being used, so race condition possible
> > Linux 3.12.48, x86_64_defconfig, GCC 4.9.1 :
> > copy_user_generic_unrolled being used, so race condition possible
> > Linux 4.2.3, x86_64_defconfig, GCC 4.6.1 : 32 bit read being used, no
> > race condition
> > Linux 4.2.3, x86_64_defconfig, GCC 4.9.1 : 32 bit read being used, no
> > race condition
> > 
> > 
> > Our idea to fix this problem is use an explicit 32 bit read in
> > get_futex_value_locked() instead of using the generic function
> > copy_from_user_inatomic() and hoping the compiler uses an atomic
> > access and the right access size.
> 
> You cannot use an explicit 32bit read. We need an access which handles
> the fault gracefully.
> 
> In current mainline this is done proper:
> 
> ret = __copy_from_user_inatomic(dst, src, size = sizeof(u32))
> 
>         __copy_from_user_nocheck(dst, src, size)
> 
>     	       if (!__builtin_constant_p(size))
>                      return copy_user_generic(dst, (__force void *)src, size);
> 	
> 	       size is constant so we end up in the switch case
> 
> 	       switch(size) {
> 	       
> 	       case 4:
> 	       	    __get_user_asm(*(u32 *)dst, (u32 __user *)src,
> 		     		   ret, "l", "k", "=r", 4);
> 		    return ret;
> ....
> 
> In 3.12 this is different:
> 
>    __copy_from_user_inatomic()
> 	copy_user_generic()
> 	    copy_user_generic_unrolled()
> 
> So this is only an issue for kernel versions < 3.13. It was fixed with
> 
> ff47ab4ff3cd: Add 1/2/4/8 byte optimization to 64bit __copy_{from,to}_user_inatomic
> 
> but nobody noticed that the race you described can happen, so it was
> never backported to the stable kernels.
> 
> @stable: Can you please pick up ff47ab4ff3cd plus 
> 
> df90ca969035d x86, sparse: Do not force removal of __user when calling copy_to/from_user_nocheck()
> 
> for stable kernels <= 3.12?
> 
> If that's too much of churn, then I can come up with an explicit fix
> for this. Let me know.

Now applied to 3.10-stable, thanks.

greg k-h