[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG_fn=XfqNjTry+kyXHVXNRX1Y7LL5=WTmAkNPzTg7XeR7DLww@mail.gmail.com>
Date: Fri, 4 Mar 2016 16:06:42 +0100
From: Alexander Potapenko <glider@...gle.com>
To: Andrey Ryabinin <ryabinin.a.a@...il.com>
Cc: Dmitry Vyukov <dvyukov@...gle.com>,
Andrey Konovalov <adech.fo@...il.com>,
Christoph Lameter <cl@...ux.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Steven Rostedt <rostedt@...dmis.org>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
JoonSoo Kim <js1304@...il.com>,
Kostya Serebryany <kcc@...gle.com>,
kasan-dev <kasan-dev@...glegroups.com>,
LKML <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH v4 5/7] mm, kasan: Stackdepot implementation. Enable
stackdepot for SLAB
On Fri, Mar 4, 2016 at 4:01 PM, Andrey Ryabinin <ryabinin.a.a@...il.com> wrote:
> 2016-03-04 17:52 GMT+03:00 Alexander Potapenko <glider@...gle.com>:
>> On Tue, Mar 1, 2016 at 12:57 PM, Andrey Ryabinin <ryabinin.a.a@...il.com> wrote:
>>>
>>>
>>> On 02/29/2016 08:12 PM, Dmitry Vyukov wrote:
>>>
>>>>>> diff --git a/lib/Makefile b/lib/Makefile
>>>>>> index a7c26a4..10a4ae3 100644
>>>>>> --- a/lib/Makefile
>>>>>> +++ b/lib/Makefile
>>>>>> @@ -167,6 +167,13 @@ obj-$(CONFIG_SG_SPLIT) += sg_split.o
>>>>>> obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
>>>>>> obj-$(CONFIG_IRQ_POLL) += irq_poll.o
>>>>>>
>>>>>> +ifeq ($(CONFIG_KASAN),y)
>>>>>> +ifeq ($(CONFIG_SLAB),y)
>>>>>
>>>>> Just try to imagine that another subsystem wants to use stackdepot. How this gonna look like?
>>>>>
>>>>> We have Kconfig to describe dependencies. So, this should be under CONFIG_STACKDEPOT.
>>>>> So any user of this feature can just do 'select STACKDEPOT' in Kconfig.
>>>>>
>>>>>> + obj-y += stackdepot.o
>>>>>> + KASAN_SANITIZE_slub.o := n
>>> _stackdepot.o
>>>
>>>
>>>>>
>>>>>> +
>>>>>> + stack->hash = hash;
>>>>>> + stack->size = size;
>>>>>> + stack->handle.slabindex = depot_index;
>>>>>> + stack->handle.offset = depot_offset >> STACK_ALLOC_ALIGN;
>>>>>> + __memcpy(stack->entries, entries, size * sizeof(unsigned long));
>>>>>
>>>>> s/__memcpy/memcpy/
>>>>
>>>> memcpy should be instrumented by asan/tsan, and we would like to avoid
>>>> that instrumentation here.
>>>
>>> KASAN_SANITIZE_* := n already takes care about this.
>>> __memcpy() is a special thing solely for kasan internals and some assembly code.
>>> And it's not available generally.
>> As far as I can see, KASAN_SANITIZE_*:=n does not guarantee it.
>> It just removes KASAN flags from GCC command line, it does not
>> necessarily replace memcpy() calls with some kind of a
>> non-instrumented memcpy().
>>
>
> With removed kasan cflags '__SANITIZE_ADDRESS__' is not defined,
> hence enable the following defines from arch/x86/include/asm/string_64.h:
>
> #if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>
> /*
> * For files that not instrumented (e.g. mm/slub.c) we
> * should use not instrumented version of mem* functions.
> */
>
> #undef memcpy
> #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
Nice!
What do you think about providing stub .c files to decouple the shared
code used by KASAN runtime from the rest of kernel?
(This is a completely different story though and can be done separately).
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Powered by blists - more mailing lists