Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757424AbeAIEN0 (ORCPT + 1 other); Mon, 8 Jan 2018 23:13:26 -0500 Received: from mail-io0-f171.google.com ([209.85.223.171]:38591 "EHLO mail-io0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753658AbeAIENX (ORCPT ); Mon, 8 Jan 2018 23:13:23 -0500 X-Google-Smtp-Source: ACJfBoskk5hfsvOub+YU7YQzW6wBqScfB+NqmEbqpFtpl/jOqXtEyb9XS6ncjM65uhbxeSEfUoaal5kjCy7ZDxjaC5k= MIME-Version: 1.0 In-Reply-To: References: <151520099201.32271.4677179499894422956.stgit@dwillia2-desk3.amr.corp.intel.com> <151520108080.32271.16420298348259030860.stgit@dwillia2-desk3.amr.corp.intel.com> <87lgh7n2tf.fsf@xmission.com> From: Linus Torvalds Date: Mon, 8 Jan 2018 20:13:20 -0800 X-Google-Sender-Auth: CzF8MBscROUEWL9gpqFzP1sLFqI Message-ID: Subject: Re: [PATCH 16/18] net: mpls: prevent bounds-check bypass via speculative execution To: Dan Williams Cc: "Eric W. Biederman" , Linux Kernel Mailing List , linux-arch@vger.kernel.org, Peter Zijlstra , Netdev , Greg KH , Thomas Gleixner , "David S. Miller" , Elena Reshetova , Alan Cox Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Mon, Jan 8, 2018 at 7:42 PM, Dan Williams wrote: > > originally from Linus and tweaked by Alexei and I: Sadly, that tweak - while clever - is wrong. > unsigned long _mask = ~(long)(_m - 1 - _i) >> BITS_PER_LONG - 1;\ Why? Because "(long)(_m-1-_i)" is not negative just because "i >= m". It can still be positive. Think "m = 100", "i=bignum". The subtraction will overflow and become positive again, the shift will shift to zero, and then the mask will become ~0. Now, you can fix it, but you need to be a tiny bit more clever. In particular, just make sure that you retain the high bit of "_i", basically making the rule be that a negative index is not ever valid. And then instead of "(_m - 1 - _i)", you use "(_i | (_m - 1 - _i))". Now the sign bit is set if _i had it set, _or_ if the subtraction turned negative, and you don't have to worry about the overflow situation. But it does require that extra step to be trustworthy. Still purely cheap arithmetic operations, although there is possibly some additional register pressure there. Somebody might be able to come up with something even more minimal (or find a fault in my fix of your tweak). Obviously, with architecture-specific code, you may well be able to do better, using the carry flag of the subtraction. For example, on x86, I think you could do it with just two instructions: # carry will be clear if idx >= max cmpq %idx,%max # mask will be clear if carry was clear, ~0 otherwise sbbq %mask,%mask to generate mask directly. I might have screwed that up. Worth perhaps trying? Linus