From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516385567; cv=none; d=google.com; s=arc-20160816; b=w9jzASTa2PDOjRzh6PwRxoiBMunnVWGnZhvzwW2FEI7VMbNgebtnD2/A454bx7gOME 5bczPB+H2bl/MqjSJBT5udAB9m4nO734eLkxbb/aaZet9qqiySqem3jTAMhKyv/IyJXl kCDXChUtkGAysiT4XoZRwvq6Xry7HpJz9d/4q1hzy1LFFHe2fNnM7m973HPGrYIWSMoO OIHjIcvfVOkeGZZNJNHVrEg/cSjYjYPovptDcnJK7qfUbbbZwA9R0JZIOVT0QkxOZfgk 5tKnKfQLTIAoRWtj8EXkABP/CMpZjwtkRNu/ugo1gTVb0Qargugh7tonv5wmHNb0lsXA XBPQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:dkim-signature:arc-authentication-results; bh=ui19pjar2t9yG30bRBJ81GGKAq2n3hydZSL9/w4fDSo=; b=PGh6Ys92EaZtwl5RSASMwhywh/NoZEs9R2LKp7blUjgWsZExy6UmV8zP3V2rXrJcPq UBjC5VIx/iv2i3OFrCS6JHaOa93+hAuHuEMjUcxwWOiB2DTElOWd6H6HWxPMvD6Ofkr2 iE0YTdOruCBROIzPd/SxvVg4PnOsSAO7C73QAn7nbOcQPG8sdnt+lV0ZxCKNHjfLIVXU MdWE0rMZgii3R4M693aQrcL7EMlwmCT0hoXvJSsHwZHdibCBC9/kXnhxqWZkTpOc0Tmj R4bYKDwLGT9q4DjhqLlXbqarWMQ5lRb+HsGJTv+N0PV16cwkiOLAEqxqlskocbMarC/p SqFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel-com.20150623.gappssmtp.com header.s=20150623 header.b=gOZFDgUa; spf=pass (google.com: domain of dan.j.williams@intel.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=dan.j.williams@intel.com Authentication-Results: mx.google.com; dkim=pass header.i=@intel-com.20150623.gappssmtp.com header.s=20150623 header.b=gOZFDgUa; spf=pass (google.com: domain of dan.j.williams@intel.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=dan.j.williams@intel.com X-Google-Smtp-Source: ACJfBovnt/ycEYthHtOO2di8+MdWpoySmxNqgwPjvlCdYl8DVZtBumgbUsNSsNUwmW7ABsciQySIwjcjgO2CqCz9o2E= MIME-Version: 1.0 In-Reply-To: References: <151632009605.21271.11304291057104672116.stgit@dwillia2-desk3.amr.corp.intel.com> <151632010687.21271.12004432287640499992.stgit@dwillia2-desk3.amr.corp.intel.com> From: Dan Williams Date: Fri, 19 Jan 2018 10:12:47 -0800 Message-ID: Subject: Re: [kernel-hardening] [PATCH v4 02/10] asm/nospec, array_ptr: sanitize speculative array de-references To: Adam Sampson Cc: Jann Horn , kernel list , linux-arch , Kernel Hardening , Catalin Marinas , "the arch/x86 maintainers" , Will Deacon , Russell King , Ingo Molnar , Greg Kroah-Hartman , "H. Peter Anvin" , Thomas Gleixner , Linus Torvalds , Andrew Morton , Alan Cox , Alexei Starovoitov Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589977446378237360?= X-GMAIL-MSGID: =?utf-8?q?1590045513544898385?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: [ adding Alexei back to the cc ] On Fri, Jan 19, 2018 at 9:48 AM, Adam Sampson wrote: > Jann Horn writes: > >>> +/* >>> + * If idx is negative or if idx > size then bit 63 is set in the mask, >>> + * and the value of ~(-1L) is zero. When the mask is zero, bounds check >>> + * failed, array_ptr will return NULL. >>> + */ >>> +#ifndef array_ptr_mask >>> +static inline unsigned long array_ptr_mask(unsigned long idx, >>> unsigned long sz) >>> +{ >>> + return ~(long)(idx | (sz - 1 - idx)) >> (BITS_PER_LONG - 1); >>> +} >>> +#endif >> >> Nit: Maybe add a comment saying that this is equivalent to >> "return ((long)idx >= 0 && idx < sz) ? ULONG_MAX : 0"? > > That's only true when sz < LONG_MAX, which is documented below but not > here; it's also different from the asm version, which doesn't do the idx > <= LONG_MAX check. So making the constraint explicit would be a good idea. > > From a bit of experimentation, when the top bit of sz is set, this > expression, the C version and the assembler version all have different > behaviour. For example, with 32-bit unsigned long: > > index=00000000 size=80000001: expr=ffffffff c=00000000 asm=ffffffff > index=80000000 size=80000001: expr=00000000 c=00000000 asm=ffffffff > index=00000000 size=a0000000: expr=ffffffff c=00000000 asm=ffffffff > index=00000001 size=a0000000: expr=ffffffff c=00000000 asm=ffffffff > index=fffffffe size=ffffffff: expr=00000000 c=00000000 asm=ffffffff > > It may be worth noting that: > > return 0 - ((long) (idx < sz)); > > causes GCC, on ia32 and amd64, to generate exactly the same cmp/sbb > sequence as in Linus's asm. Are there architectures where this form > would allow speculation? We're operating on the assumption that compilers will not try to introduce branches where they don't exist in the code, so if this is producing identical assembly I think we should go with it and drop the x86 array_ptr_mask.