Messages in this thread | | | From | Dmitry Vyukov <> | Date | Tue, 19 Sep 2017 15:47:08 +0200 | Subject | Re: [PATCH 1/3] kcov: remove #ifdef CONFIG_RANDOMIZE_BASE |
| |
On Tue, Sep 19, 2017 at 3:30 PM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Tue, Sep 19, 2017 at 03:46:46PM +0300, Andrey Ryabinin wrote: >> There is no need to surround kaslr_offset() with CONFIG_RANDOMIZE_BASE ifdef. >> kaslr_offset() will just return 0 if CONFIG_RANDOMIZE_BASE isn't set. >> >> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com> >> --- >> kernel/kcov.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/kernel/kcov.c b/kernel/kcov.c >> index 3f693a0f6f3e..2f0e7a7c7afc 100644 >> --- a/kernel/kcov.c >> +++ b/kernel/kcov.c >> @@ -69,9 +69,7 @@ void notrace __sanitizer_cov_trace_pc(void) >> unsigned long pos; >> unsigned long ip = _RET_IP_; >> >> -#ifdef CONFIG_RANDOMIZE_BASE >> ip -= kaslr_offset(); >> -#endif > > I think this is sound, but as Dmitry points out it'll mean we do some > pointless work. For example on arm64 we have: > > static inline unsigned long kaslr_offset(void) > { > return kimage_vaddr - KIMAGE_VADDR; > } > > ... where kimage_vaddr is a global variable, and KIMAGE_VADDR is a > constant (and should be identical for !CONFIG_RANDOMIZE_BASE kernels). > > I think it would be reasonable to make that: > > static inline unsigned long kaslr_offset(void) > { > if (!IS_ENABLED(CONFIG_RANDOMIZE_BASE)) > return 0; > > return kimage_vaddr - KIMAGE_VADDR; > } > > ... and simplify callers as above.
Sounds reasonable to me.
| |