Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp2982422yba; Tue, 16 Apr 2019 02:03:07 -0700 (PDT) X-Google-Smtp-Source: APXvYqyFrejXVRSZdOc6ChL8JRZRkYF1inigIgyd9tfDw+q9GMcigHBghG5AlGoObYF1YEGNqHnS X-Received: by 2002:a63:df12:: with SMTP id u18mr75804485pgg.135.1555405387154; Tue, 16 Apr 2019 02:03:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1555405387; cv=none; d=google.com; s=arc-20160816; b=kA3Jd7NI+qq9+9ozRC40OQt1/yeOKlcsmKjH6Q6BeAcG5fsoPtWqj576WpXSovZNeZ 6Ob+V4fLRKEq9N2T5mxFjKG6jWN6VsCWXxJCOy5hOV7CJ+T+J/CktgvEGsT2yTSJW0ro jsaldxBevJVuDrhblhuV4h9hY+xUF6uQ02RerF4oREn7640JCECye9RiZI+oUGJa9xnR 7+33YzWvJ3nOn5SsjxizUV2EbGUJZ+Q3rnwxVOVepR+VYZFaDDKAY7SoccJoB5Jb8NMw 1C4Bdzb3GBT4t7Iw0ivmdHXgNpYaJer0z/qiCEyPPc9jQ5WLE0g1XMz8OCtVwDK943s4 9dig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=HsLlj69yDALe9staB83zogA+TuNWH2GuI0j5XUCpc8k=; b=jSbvDJel2sZf+KgaGmW5oYJmStxmxMo5MPmucohsvwQT/eCOTgFhrzxcxDmIh/r1g6 g3G6hn9t5QekfOvsnKDbAtiTQW5e/6zxNLjlY0Um040a4jvUAZNF+2GWlSZ45xLzDSB+ ZyuQsrA7ugXq3T4yyXQ8oE0S4IYsr8r5CAyr/mVW7GViv2A+quUOF4/pc6cC7uJmY4YW wpdcYfQBs6cKEcPn7Dabp1qfPr+TXFOXw2lF7AT8nwtKeCxHn6Xs/quBVKBiMRKZWbDq MK9XTPDbfJNSfq3Sw0QSZvlfhGu5xEqwKeeLNutx8m6QKUqknwRvSo41Nbu8BDXsT2gT NGsg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=WE5BA9wd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q7si40988541pgh.541.2019.04.16.02.02.50; Tue, 16 Apr 2019 02:03:07 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=WE5BA9wd; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728722AbfDPJA4 (ORCPT + 99 others); Tue, 16 Apr 2019 05:00:56 -0400 Received: from mail.kernel.org ([198.145.29.99]:34128 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726576AbfDPJAz (ORCPT ); Tue, 16 Apr 2019 05:00:55 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 08C332073F; Tue, 16 Apr 2019 09:00:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1555405254; bh=75Z0kfiikmN147mGRxteAYOU4Z5/vwQW9tZ968CQdAk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=WE5BA9wdUUpil/74XovX+oJA6nhCwvjF2i3rX6hUz5CndY1tKwAqHZFU60LsSTYAS K2pvn6HNKKfkwwHMxoX6thMxn0VUfuyeq0fvtoW2h5QjHdbJuLY2OiQv1aIGTPHrSH ZWEMoeQAWiXQeHHXpxhKAw99Nbtmy0bSXnNp4CTE= Date: Tue, 16 Apr 2019 11:00:52 +0200 From: Greg Kroah-Hartman To: Nathan Chancellor Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, stable@kernel.org, Will Deacon Subject: Re: [PATCH 4.9 72/76] arm64: futex: Fix FUTEX_WAKE_OP atomic ops with non-zero result value Message-ID: <20190416090052.GD4209@kroah.com> References: <20190415183707.712011689@linuxfoundation.org> <20190415183729.170980546@linuxfoundation.org> <20190415220151.GA23056@archlinux-i9> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190415220151.GA23056@archlinux-i9> User-Agent: Mutt/1.11.4 (2019-03-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 15, 2019 at 03:01:51PM -0700, Nathan Chancellor wrote: > On Mon, Apr 15, 2019 at 08:44:36PM +0200, Greg Kroah-Hartman wrote: > > From: Will Deacon > > > > commit 045afc24124d80c6998d9c770844c67912083506 upstream. > > > > Rather embarrassingly, our futex() FUTEX_WAKE_OP implementation doesn't > > explicitly set the return value on the non-faulting path and instead > > leaves it holding the result of the underlying atomic operation. This > > means that any FUTEX_WAKE_OP atomic operation which computes a non-zero > > value will be reported as having failed. Regrettably, I wrote the buggy > > code back in 2011 and it was upstreamed as part of the initial arm64 > > support in 2012. > > > > The reasons we appear to get away with this are: > > > > 1. FUTEX_WAKE_OP is rarely used and therefore doesn't appear to get > > exercised by futex() test applications > > > > 2. If the result of the atomic operation is zero, the system call > > behaves correctly > > > > 3. Prior to version 2.25, the only operation used by GLIBC set the > > futex to zero, and therefore worked as expected. From 2.25 onwards, > > FUTEX_WAKE_OP is not used by GLIBC at all. > > > > Fix the implementation by ensuring that the return value is either 0 > > to indicate that the atomic operation completed successfully, or -EFAULT > > if we encountered a fault when accessing the user mapping. > > > > Cc: > > Fixes: 6170a97460db ("arm64: Atomic operations") > > Signed-off-by: Will Deacon > > Signed-off-by: Greg Kroah-Hartman > > > > --- > > arch/arm64/include/asm/futex.h | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > --- a/arch/arm64/include/asm/futex.h > > +++ b/arch/arm64/include/asm/futex.h > > @@ -33,8 +33,8 @@ > > " prfm pstl1strm, %2\n" \ > > "1: ldxr %w1, %2\n" \ > > insn "\n" \ > > -"2: stlxr %w3, %w0, %2\n" \ > > -" cbnz %w3, 1b\n" \ > > +"2: stlxr %w0, %w3, %2\n" \ > > +" cbnz %w0, 1b\n" \ > > " dmb ish\n" \ > > "3:\n" \ > > " .pushsection .fixup,\"ax\"\n" \ > > @@ -53,29 +53,29 @@ > > static inline int > > arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr) > > { > > - int oldval = 0, ret, tmp; > > + int oldval, ret, tmp; > > > > pagefault_disable(); > > > > switch (op) { > > case FUTEX_OP_SET: > > - __futex_atomic_op("mov %w0, %w4", > > + __futex_atomic_op("mov %w3, %w4", > > ret, oldval, uaddr, tmp, oparg); > > break; > > case FUTEX_OP_ADD: > > - __futex_atomic_op("add %w0, %w1, %w4", > > + __futex_atomic_op("add %w3, %w1, %w4", > > ret, oldval, uaddr, tmp, oparg); > > break; > > case FUTEX_OP_OR: > > - __futex_atomic_op("orr %w0, %w1, %w4", > > + __futex_atomic_op("orr %w3, %w1, %w4", > > ret, oldval, uaddr, tmp, oparg); > > break; > > case FUTEX_OP_ANDN: > > - __futex_atomic_op("and %w0, %w1, %w4", > > + __futex_atomic_op("and %w3, %w1, %w4", > > ret, oldval, uaddr, tmp, ~oparg); > > break; > > case FUTEX_OP_XOR: > > - __futex_atomic_op("eor %w0, %w1, %w4", > > + __futex_atomic_op("eor %w3, %w1, %w4", > > ret, oldval, uaddr, tmp, oparg); > > break; > > default: > > > > > > This causes a (false) build warning with AOSP's GCC 4.9.4 (which is > used to build nearly all arm64 Android kernels before 4.14): > > CC kernel/futex.o > ../kernel/futex.c: In function 'do_futex': > ../kernel/futex.c:1492:17: warning: 'oldval' may be used uninitialized in this function [-Wmaybe-uninitialized] > return oldval == cmparg; > ^ > In file included from ../kernel/futex.c:69:0: > ../arch/arm64/include/asm/futex.h:56:6: note: 'oldval' was declared here > int oldval, ret, tmp; > ^ > > The only reason I bring this up is Qualcomm based kernels have a Python > script that emulates -Werror, meaning this will be fatal for a large > number of kernels, when this eventually gets merged into them. Argh, really? That's a buggy compiler that you have there, as oldval will be set correctly if all is good, and if not, ret will be and the code will error out. Working around broken compilers is not something I really like doing :( That being said, does this also show up in the 4.19.y and 5.0.y tree right now? If not, why not? thanks, greg k-h