2015-06-11 16:39 GMT+03:00 Linus Walleij [off-list ref]:

On Fri, May 15, 2015 at 3:59 PM, Andrey Ryabinin [off-list ref] wrote:

quoted

This patch adds arch specific code for kernel address sanitizer
(see Documentation/kasan.txt).

I looked closer at this again ... I am trying to get KASan up for
ARM(32) with some tricks and hacks.

I have some patches for that. They still need some polishing, but works for me.
I could share after I get back to office on Tuesday.

quoted

+config KASAN_SHADOW_OFFSET
+       hex
+       default 0xdfff200000000000 if ARM64_VA_BITS_48
+       default 0xdffffc8000000000 if ARM64_VA_BITS_42
+       default 0xdfffff9000000000 if ARM64_VA_BITS_39

So IIUC these offsets are simply chosen to satisfy the equation

        SHADOW = (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
                + KASAN_SHADOW_OFFSET;

For all memory that needs to be covered, i.e. kernel text+data,
modules text+data, any other kernel-mode running code+data.

And it needs to be statically assigned like this because the offset
is used at compile time.

Atleast that is how I understood it... correct me if wrong.
(Dunno if all is completely obvious to everyone else in the world...)

I think you understood this right.

quoted

+/*
+ * KASAN_SHADOW_START: beginning of the kernel virtual addresses.
+ * KASAN_SHADOW_END: KASAN_SHADOW_START + 1/8 of kernel virtual addresses.
+ */
+#define KASAN_SHADOW_START      (UL(0xffffffffffffffff) << (VA_BITS))
+#define KASAN_SHADOW_END        (KASAN_SHADOW_START + (1UL << (VA_BITS - 3)))

Will this not mean that shadow start to end actually covers *all*
virtual addresses including userspace and what not? However a
large portion of this shadow memory will be unused because the
SHADOW_OFFSET only works for code compiled for the kernel
anyway.

SHADOW_OFFSET:SHADOW_END - covers *all* 64bits of virtual addresses.
SHADOW_OFFSET:SHADOW_START - unused shadow.
SHADOW_START:SHADOW_END - covers only kernel virtual addresses (used shadow).

When working on ARM32 I certainly could not map
(1UL << (VA_BITS -3)) i.e. for 32 bit (1UL << 29) bytes (512 MB) of

Why not? We can just take it from TASK_SIZE.

virtual memory for shadow, instead I had to restrict it to the size that
actually maps the memory used by the kernel.

That should work too.

I tried shrinking it down but it crashed on me so tell me how
wrong I am ... :)

But here comes the real tricks, where I need some help to
understand the patch set, maybe some comments should be
inserted here and there to ease understanding:

...

quoted

+
+void __init kasan_early_init(void)
+{
+       kasan_map_early_shadow(swapper_pg_dir);
+       start_kernel();
+}

So as far as I can see, kasan_early_init() needs to be called before
we even run start_kernel() because every memory access would
crash unless the MMU is set up for the shadow memory.

 kasan_early_init() should be called before *any* instrumented function.

Is it correct that when the pte's, pgd's and pud's are populated
KASan really doesn't kick in, it's just done to have some scratch
memory with whatever contents so as to do dummy updates
for the __asan_loadN() and __asan_storeN() calls, and no checks
start until the shadow memory is populated in kasan_init()
i.e. there are no KASan checks for any code executing up
to that point, just random writes and reads?

Yes, kasan_early_init() setups scratch memory with whatever contents.
But  KASan checks shadow before kasan_init(), that's the reason why we
need scratch shadow.
So checks are performed, but KASan don't print any reports, because
init_task has non-zero kasan_depth flag (see include/linux/init_task.h)
We check that flag in kasan_report() and print report iff it have zero value.

In kasan_init() after shadow populated, we enable reports by setting
kasan_depth to zero.

Also, this code under kasan_early_init(), must that not be
written extremely carefully to avoid any loads and stores?
I.e. even if this file is obviously compiled with
KASAN_SANITIZE_kasan_init.o := n so that it is not
instrumented, I'm thinking about the functions it is calling,
like set_pte(), pgd_populate(), pmd_offset() etc etc.

These functions are not instrumented as well, because they are static
and located in headers.
So these functions will be in kasan_init.o translation unit and will
be compiled without -fsanitize=kernel-address.

Are we just lucky that these functions never do any loads
and stores?

We relay on fact that these functions are static inline and do not call other
functions from other (instrumented) files.