From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751450AbdJEHid (ORCPT ); Thu, 5 Oct 2017 03:38:33 -0400 Received: from mail.skyhub.de ([5.9.137.197]:33636 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751270AbdJEHic (ORCPT ); Thu, 5 Oct 2017 03:38:32 -0400 Date: Thu, 5 Oct 2017 09:38:09 +0200 From: Borislav Petkov To: Mathias Krause Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Borislav Petkov , x86@kernel.org, linux-kernel@vger.kernel.org, Michael Matz Subject: Re: [PATCH] x86/alternatives: Fix alt_max_short macro to really be a max() Message-ID: <20171005073809.depam2y4vw2mb5ya@pd.tnic> References: <1507140492-7832-1-git-send-email-minipli@googlemail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1507140492-7832-1-git-send-email-minipli@googlemail.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 04, 2017 at 08:08:12PM +0200, Mathias Krause wrote: > The alt_max_short() macro in asm/alternative.h does not work as > intended, leading to nasty bugs. E.g. alt_max_short("1", "3") > evaluates to 3, but alt_max_short("3", "1") evaluates to 1 -- not > exactly the maximum of 1 and 3. > > In fact, I had to learn it the hard way by crashing my kernel in not > so funny ways by attempting to make use of the ALTENATIVE_2 macro > with alternatives where the first one was larger than the second > one. > > According to [1] and commit dbe4058a6a44 ("x86/alternatives: Fix > ALTERNATIVE_2 padding generation properly") the right handed side > should read "-(-(a < b))" not "-(-(a - b))". Fix that, to make the > macro work as intended. > > While at it, fix up the comment regarding the additional "-", too. > It's not about gas' usage of s32 but brain dead logic of having a > "true" value of -1 for the < operator ... *sigh* > > Btw., the one in asm/alternative-asm.h is correct. And, apparently, > all current users of ALTERNATIVE_2() pass same sized alternatives, > avoiding to hit the bug. > > [1] http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax > > Fixes: dbe4058a6a44 ("x86/alternatives: Fix ALTERNATIVE_2 padding generation properly") > Signed-off-by: Mathias Krause > Cc: Borislav Petkov > --- > arch/x86/include/asm/alternative.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > index c096624137ae..7c553f48f163 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -106,9 +106,9 @@ static inline int alternatives_text_reserved(void *start, void *end) > * max without conditionals. Idea adapted from: ^^^^^^^^^^^^^^^^^^^^^^^^^ You did read this part, right? AFAIR, gas can't stomach conditionals but I don't remember the details anymore. Could be that -1 representation of "true". Let me add Micha to CC. Anyway, how can I reproduce your observation? Code snippet and compiler pls. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.