On 09/19/2017 04:47 PM, Dmitry Vyukov wrote: > On Tue, Sep 19, 2017 at 3:30 PM, Mark Rutland <[email protected]> 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 <[email protected]> >>> --- >>> 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. >
As kbuilt robot pointed out, we would also need to introduce generic kaslr_offset() for arches that don't have it. I seem like too much trouble for very little gain, so I'll keep current code as is.
