Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752159AbbGRP5S (ORCPT ); Sat, 18 Jul 2015 11:57:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59321 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751437AbbGRP5R (ORCPT ); Sat, 18 Jul 2015 11:57:17 -0400 Date: Sat, 18 Jul 2015 10:57:14 -0500 From: Josh Poimboeuf To: Borislav Petkov Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Michal Marek , Peter Zijlstra , Andy Lutomirski , Linus Torvalds , Andi Kleen , Pedro Alves , x86@kernel.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*() Message-ID: <20150718155714.GB19968@treble.redhat.com> References: <0d8517b7ab757e00a13b3abe2b677d9eb23362be.1437150175.git.jpoimboe@redhat.com> <20150717171726.GA21568@nazgul.tnic> <20150717173220.GA12761@treble.redhat.com> <20150718050536.GA23478@nazgul.tnic> <20150718134415.GA24587@treble.redhat.com> <20150718145629.GB30713@nazgul.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150718145629.GB30713@nazgul.tnic> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3129 Lines: 71 On Sat, Jul 18, 2015 at 04:56:29PM +0200, Borislav Petkov wrote: > On Sat, Jul 18, 2015 at 08:44:15AM -0500, Josh Poimboeuf wrote: > > Ok, so would you rather adding a whitelist to tell stackvalidate to > > ignore it? Something like this? > > I tried it and maybe I'm missing something but that doesn't work: > > $ make drivers/gpu/drm/i915/intel_ringbuffer.o > CHK include/config/kernel.release > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > CHK include/generated/timeconst.h > CHK include/generated/bounds.h > CHK include/generated/asm-offsets.h > CALL scripts/checksyscalls.sh > CC drivers/gpu/drm/i915/intel_ringbuffer.o > ./arch/x86/include/asm/arch_hweight.h: Assembler messages: > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined > scripts/Makefile.build:258: recipe for target 'drivers/gpu/drm/i915/intel_ringbuffer.o' failed > make[1]: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 1 > Makefile:1528: recipe for target 'drivers/gpu/drm/i915/intel_ringbuffer.o' failed > make: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 2 Yeah, it doesn't actually support this particular example yet. I was just trying to figure out if that's what you were proposing. > Also, that label temp32 could be more descriptive. Yeah, that's from: ".Ltemp" __stringify(__LINE__) ":;" Which was intended to give a unique ID for each use of the macro, but apparently that didn't work as planned here. > so you see that a CALL instruction gets replaced with a POPCNT and > the feature bit used is 4*32+23 which is X86_FEATURE_POPCNT. This > information is enough to detect that particular case and add the offset > ".long 661b - ." to the list of instructions which stackvalidate should > ignore. Currently, when stackvalidate sees an ALTERNATIVE, it assumes that either code path is possible, so it follows both paths in parallel. If I understand right, you're proposing that stackvalidate should only follow the POPCNT path and never follow the !POPCNT path? > Anyway, this is what I'd do. > > IMNSVHO, we must be very conservative and not add some > markers/helpers/etc to code only so that tools can do their job. Not if > it can be helped. Instead, tools should do the hard work and we should > keep kernel code clean. In general, I agree, and I like the original patch much better. IMO, it achieved the goal of keeping the kernel code clean, while fixing the frame pointer bug. If you insist on breaking stack traces on !POPCNT, I can probably add some intelligence to stackvalidate to look for !POPCNT and ignore it. It seems less "clean" to me, though. -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/