linux-kernel - Re: [PATCH 1/3] futex: remove duplicated code

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]

Date:   Thu, 9 Mar 2017 17:37:43 -0500
From:   Rich Felker <dalias@...c.org>
To:     "H. Peter Anvin" <hpa@...or.com>
Cc:     Rob Landley <rob@...dley.net>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Jiri Slaby <jslaby@...e.cz>, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, Richard Henderson <rth@...ddle.net>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>,
        Vineet Gupta <vgupta@...opsys.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Richard Kuo <rkuo@...eaurora.org>,
        Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Michal Simek <monstr@...str.eu>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Jonas Bonn <jonas@...thpole.se>,
        Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
        Stafford Horne <shorne@...il.com>,
        "James E.J. Bottomley" <jejb@...isc-linux.org>,
        Helge Deller <deller@....de>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Martin Schwidefsky <schwidefsky@...ibm.com>,
        Heiko Carstens <heiko.carstens@...ibm.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        "David S. Miller" <davem@...emloft.net>, Chris.Metcalf@...or.com
Subject: Re: [PATCH 1/3] futex: remove duplicated code

On Wed, Mar 08, 2017 at 08:36:30PM -0800, H. Peter Anvin wrote:
> <cmetcalf@...lanox.com>,Thomas Gleixner <tglx@...utronix.de>,Ingo Molnar <mingo@...hat.com>,Chris Zankel <chris@...kel.net>,Max Filippov <jcmvbkbc@...il.com>,Arnd Bergmann <arnd@...db.de>,x86@...nel.org,linux-alpha@...r.kernel.org,linux-snps-arc@...ts.infradead.org,linux-arm-kernel@...ts.infradead.org,linux-hexagon@...r.kernel.org,linux-ia64@...r.kernel.org,linux-mips@...ux-mips.org,openrisc@...ts.librecores.org,linux-parisc@...r.kernel.org,linuxppc-dev@...ts.ozlabs.org,linux-s390@...r.kernel.org,linux-sh@...r.kernel.org,sparclinux@...r.kernel.org,linux-xtensa@...ux-xtensa.org,linux-arch@...r.kernel.org
> From: hpa@...or.com
> Message-ID: <83324528-AAA1-4BED-B0C7-48426ECBA261@...or.com>
> 
> On March 8, 2017 8:16:49 PM PST, Rob Landley <rob@...dley.net> wrote:
> >On 03/04/2017 07:05 AM, Russell King - ARM Linux wrote:
> >> On Fri, Mar 03, 2017 at 01:27:10PM +0100, Jiri Slaby wrote:
> >>> diff --git a/kernel/futex.c b/kernel/futex.c
> >>> index b687cb22301c..c5ff9850952f 100644
> >>> --- a/kernel/futex.c
> >>> +++ b/kernel/futex.c
> >>> @@ -1457,6 +1457,42 @@ futex_wake(u32 __user *uaddr, unsigned int
> >flags, int nr_wake, u32 bitset)
> >>>  	return ret;
> >>>  }
> >>>  
> >>> +static int futex_atomic_op_inuser(int encoded_op, u32 __user
> >*uaddr)
> >>> +{
> >>> +	int op = (encoded_op >> 28) & 7;
> >>> +	int cmp = (encoded_op >> 24) & 15;
> >>> +	int oparg = (encoded_op << 8) >> 20;
> >>> +	int cmparg = (encoded_op << 20) >> 20;
> >> 
> >> Hmm.  oparg and cmparg look like they're doing these shifts to get
> >sign
> >> extension of the 12-bit values by assuming that "int" is 32-bit -
> >> probably worth a comment, or for safety, they should be "s32" so it's
> >> not dependent on the bit-width of "int".
> >
> >I thought Linux depended on the LP64 standard for all architectures?
> >
> >Standard: http://www.unix.org/whitepapers/64bit.html
> >Rationale: http://www.unix.org/version2/whatsnew/lp64_wp.html
> >
> >So int has a defined bit width (32) on linux?
> >
> >Rob
> 
> Linux is ILP32 on 32-bit architectures and LP64 on 64-bit
> architectures, but that doesn't inherently make this stuff clear.

32-bit int is part of the futex ABI anyway, but that doesn't justify
gratuitous assumption of it in other places where the assumption and
the intent may not be clear to the reader. It also doesn't justify all
the UB from invalid shifts in the above code.

Rich