Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1946081AbbDXVzd (ORCPT ); Fri, 24 Apr 2015 17:55:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59645 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966933AbbDXVzb (ORCPT ); Fri, 24 Apr 2015 17:55:31 -0400 Date: Fri, 24 Apr 2015 16:55:26 -0500 From: Josh Poimboeuf To: Jiri Kosina Cc: Seth Jennings , Vojtech Pavlik , live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] livepatch: x86: make kASLR logic more accurate Message-ID: <20150424215526.GB27123@treble.redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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: 3168 Lines: 85 On Fri, Apr 24, 2015 at 09:59:03PM +0200, Jiri Kosina wrote: > We give up old_addr hint from the coming patch module in cases when kernel load > base has been randomized (as in such case, the coming module has no idea about > the exact randomization offset). > > We are currently too pessimistic, and give up immediately as soon as > CONFIG_RANDOMIZE_BASE is set; this doesn't however directly imply that the > load base has actually been randomized. There are config options that > disable kASLR (such as hibernation), user could have disabled kaslr on > kernel command-line, etc. > > The loader propagates the information whether kernel has been randomized > through bootparams. This allows us to have the condition more accurate. > > On top of that, it seems unnecessary to give up old_addr hints even if > randomization is active. The relocation offset can be computed as > difference between _text start and __START_KERNEL, and therefore old_addr > can be adjusted accordingly. > > Signed-off-by: Jiri Kosina > --- > > v1 -> v2: I accidentally used kgr_ suffix (which we use in kGraft) instead > of klp_ in v1. > > arch/x86/include/asm/livepatch.h | 4 ++++ > arch/x86/kernel/livepatch.c | 5 +++++ > kernel/livepatch/core.c | 5 +++-- > 3 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h > index 2d29197..84a3247 100644 > --- a/arch/x86/include/asm/livepatch.h > +++ b/arch/x86/include/asm/livepatch.h > @@ -23,8 +23,12 @@ > > #include > #include > +#include > > #ifdef CONFIG_LIVEPATCH > + > +extern unsigned long klp_vmlinux_relocation_offset(void); > + > static inline int klp_check_compiler_support(void) > { > #ifndef CC_USING_FENTRY > diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c > index ff3c3101d..6df7902 100644 > --- a/arch/x86/kernel/livepatch.c > +++ b/arch/x86/kernel/livepatch.c > @@ -88,3 +88,8 @@ int klp_write_module_reloc(struct module *mod, unsigned long type, > > return ret; > } > + > +unsigned long klp_vmlinux_relocation_offset(void) > +{ > + return (unsigned long)&_text - __START_KERNEL; > +} > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 284e269..ff4c35c 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -234,8 +234,9 @@ static int klp_find_verify_func_addr(struct klp_object *obj, > int ret; > > #if defined(CONFIG_RANDOMIZE_BASE) > - /* KASLR is enabled, disregard old_addr from user */ > - func->old_addr = 0; > + /* If KASLR has been enabled, adjust old_addr accordingly */ > + if (kaslr_enabled()) > + func->old_addr += klp_vmlinux_relocation_offset(); > #endif Can we remove the #ifdef now? It would probably be better to have an #ifdef in asm/setup.h for the kaslr_enabled() definition. -- 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/