From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516357269; cv=none; d=google.com; s=arc-20160816; b=cg9WXN02IzUvnTRknCGWJF2MB4GeL+SzWLAjJPJGPODdPauIokwEBRI7fpNZbfr5/v fnzwYosytxC29DmnRqzR0RyshDgTHEVlfPCx3monMjtzp0aXp/RBcJOkVL36nmFPz82r 37hohchrCMrBrYJLYepIkOU8SviLcqXRAeSVOU5sPd+Rjb/+8QKCWktj7UpYxBRd0sPj UkvUMPgidQIZ3Lr2vchkgrrq5MQ09L6Sbr+M4BZqyKNdVAvb3nLmfr2DKAa8GUw2guPa uDnTwuWYiKNMxZShxYDKt01cIvCdRntcfaNUpcaEh9qMxNZGPR6R2KgW4cyLHIJplBOu mMLA== 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=cd+cyC5XfoYlvK5mY/Ds7ONDJnJOSrGYPeORX58979w=; b=pW4V/o9bxfN2aZc+Vkbp12Vn2nPu8vzv6vRnG8pOBV2j3kLS8EREDi7hN9zI8HHM69 cbSb1RBCeMemJpuC08wLFd5aLD3o+zHlWuDrIO/ca/WmyOgucM/79aZRpgGD3N4B5ko5 9m5dOE1TGh/1zwlPmb1koYzbuJrj6Z2e5JZGcOx1XeR1jad14JTU27almscr45tZVCO9 AkZeixwmIewhRTGD98/FWS28K0/TkCTw/JM4drSlIOKsF8jNARMHO09w6oBOHh4UE651 H3I5iMiDPfHjPeZx1rs7YbmtmSefqfBeCbUPXpZQqV2MX3qN9bL+uESfgaKatXw9IzO1 aLDQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=sN5wz7z5; spf=pass (google.com: domain of jannh@google.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=sN5wz7z5; spf=pass (google.com: domain of jannh@google.com designates 209.85.220.41 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com X-Google-Smtp-Source: ACJfBosuYa7uGiVFaeJUsWPMTHeX+/OPCN3kGn6GNjJGMQVOEUL9wIDifWnoSKKmVQv1u9pC/yMrcDFdmAyiEXNYSIA= MIME-Version: 1.0 In-Reply-To: <151632010687.21271.12004432287640499992.stgit@dwillia2-desk3.amr.corp.intel.com> References: <151632009605.21271.11304291057104672116.stgit@dwillia2-desk3.amr.corp.intel.com> <151632010687.21271.12004432287640499992.stgit@dwillia2-desk3.amr.corp.intel.com> From: Jann Horn Date: Fri, 19 Jan 2018 11:20:48 +0100 Message-ID: Subject: Re: [kernel-hardening] [PATCH v4 02/10] asm/nospec, array_ptr: sanitize speculative array de-references To: Dan Williams Cc: 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@linux.intel.com 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?1590015840030458375?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Fri, Jan 19, 2018 at 1:01 AM, Dan Williams wrote: > 'array_ptr' is proposed as a generic mechanism to mitigate against > Spectre-variant-1 attacks, i.e. an attack that bypasses boundary checks > via speculative execution). The 'array_ptr' implementation is expected > to be safe for current generation cpus across multiple architectures > (ARM, x86). > > Based on an original implementation by Linus Torvalds, tweaked to remove > speculative flows by Alexei Starovoitov, and tweaked again by Linus to > introduce an x86 assembly implementation for the mask generation. [...] > +/* > + * 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"? > +/** > + * array_ptr - Generate a pointer to an array element, ensuring > + * the pointer is bounded under speculation to NULL. > + * > + * @base: the base of the array > + * @idx: the index of the element, must be less than LONG_MAX > + * @sz: the number of elements in the array, must be less than LONG_MAX > + * > + * If @idx falls in the interval [0, @sz), returns the pointer to > + * @arr[@idx], otherwise returns NULL. > + */ > +#define array_ptr(base, idx, sz) \ > +({ \ > + union { typeof(*(base)) *_ptr; unsigned long _bit; } __u; \ > + typeof(*(base)) *_arr = (base); \ > + unsigned long _i = (idx); \ > + unsigned long _mask = array_ptr_mask(_i, (sz)); \ > + \ > + __u._ptr = _arr + (_i & _mask); \ > + __u._bit &= _mask; \ AFAICS, if `idx` is out of bounds, you first zero out the index (`_i & _mask`) and then immediately afterwards zero out the whole pointer (`_u._bit &= _mask`). Is there a reason for the `_i & _mask`, and if so, can you add a comment explaining that?