On 2/15/19 1:10 PM, Christophe Leroy wrote:
>
>
> Le 15/02/2019 à 11:01, Andrey Ryabinin a écrit :
>>
>>
>> On 2/15/19 11:41 AM, Christophe Leroy wrote:
>>>
>>>
>>> Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
>>>> Hi Christophe,
>>>>
>>>>> --- a/arch/powerpc/include/asm/string.h
>>>>> +++ b/arch/powerpc/include/asm/string.h
>>>>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void
>>>>> *,__kernel_size_t);
>>>>> extern void * memchr(const void *,int,__kernel_size_t);
>>>>> extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>>>> +void *__memset(void *s, int c, __kernel_size_t count);
>>>>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>>>>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>>>>> +
>>>>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>>>>> +/*
>>>>> + * For files that are not instrumented (e.g. mm/slub.c) we
>>>>> + * should use not instrumented version of mem* functions.
>>>>> + */
>>>>> +#define memcpy(dst, src, len) __memcpy(dst, src, len)
>>>>> +#define memmove(dst, src, len) __memmove(dst, src, len)
>>>>> +#define memset(s, c, n) __memset(s, c, n)
>>>>> +#endif
>>>>> +
>>>>
>>>> I'm finding that I miss tests like 'kasan test: kasan_memcmp
>>>> out-of-bounds in memcmp' because the uninstrumented asm version is used
>>>> instead of an instrumented C version. I ended up guarding the relevant
>>>> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
>>>> the arch versions if we're not compiled with KASAN.
>>>>
>>>> I find I need to guard and unexport strncpy, strncmp, memchr and
>>>> memcmp. Do you need to do this on 32bit as well, or are those tests
>>>> passing anyway for some reason?
>>>
>>> Indeed, I didn't try the KASAN test module recently, because my configs
>>> don't have CONFIG_MODULE by default.
>>>
>>> Trying to test it now, I am discovering that module loading oopses with
>>> latest version of my series, I need to figure out exactly why. Here below
>>> the oops by modprobing test_module (the one supposed to just say hello to
>>> the world).
>>>
>>> What we see is an access to the RO kasan zero area.
>>>
>>> The shadow mem is 0xf7c00000..0xffc00000
>>> Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
>>> 0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.
>>>
>>> Why is kasan trying to access that ? Isn't kasan supposed to not check
>>> stuff in vmalloc area ?
>>
>> It tries to poison global variables in modules. If module is in vmalloc,
>> than it will try to poison vmalloc.
>> Given that the vmalloc area is not so big on 32bits, the easiest solution is
>> to cover all vmalloc with RW shadow.
>>
>
> Euh ... Not so big ?
>
> Memory: 96448K/131072K available (8016K kernel code, 1680K rwdata
> , 2720K rodata, 624K init, 4678K bss, 34624K reserved, 0K cma-reserved)
> Kernel virtual memory layout:
> * 0xffefc000..0xffffc000 : fixmap
> * 0xf7c00000..0xffc00000 : kasan shadow mem
> * 0xf7a00000..0xf7c00000 : consistent mem
> * 0xf7a00000..0xf7a00000 : early ioremap
> * 0xc9000000..0xf7a00000 : vmalloc & ioremap
>
> Here, vmalloc area size 0x2ea00000, that is 746Mbytes. Shadow for this would
> be 93Mbytes and we are already using 16Mbytes to shadow the linear memory
> area .... this poor board has 128Mbytes RAM in total.
>
> So another solution is needed.
>
Ok.
As a temporary workaround your can make __asan_register_globals() to skip
globals in vmalloc().
Obviously it means that out-of-bounds accesses to in modules will be missed.
Non temporary solution would making kasan to fully support vmalloc, i.e. remove
RO shadow and allocate/free shadow on vmalloc()/vfree().
But this feels like separate task, out of scope of this patch set.
It is also possible to follow some other arches - dedicate separate address
range for modules, allocate/free shadow in module_alloc/free.
But it doesn't seem worthy to implement this only for the sake of kasan, since
vmalloc support needs to be done anyway.
