Re: [PATCHv4] Enable -fsanitize-recover for KASan

[Dmitry Vyukov <dvyukov at google dot com>]

On Fri, Oct 24, 2014 at 12:28 PM, Yury Gribov <y.gribov@samsung.com> wrote:
> On 10/23/2014 03:10 PM, Andrey Ryabinin wrote:
>>
>> On 10/23/2014 02:38 PM, Jakub Jelinek wrote:
>>>
>>> On Thu, Oct 23, 2014 at 02:33:42PM +0400, Yury Gribov wrote:
>>>>
>>>> Actually this is a historical artifact.  If inlining proves to be
>>>> significantly faster, they may want to switch.
>>>
>>>
>>> Ok.
>>>
>>>>> So, at that point you can include your ugly hacks in __asan_load* logic
>>>>> in
>>>>> the kernel, the difference between __asan_load4 and
>>>>> __asan_load4_noabort
>>>>> will be just that the latter will always return, while the former will
>>>>> not
>>>>> if some error has been reported.
>>>>> All the __asan_load* and __asan_store* entrypoints, regardless of
>>>>> -f{,no-}sanitize-recover=kernel-address are by definition not noreturn,
>>>>> they
>>>>> in the common case (if the code is not buggy) return.
>>>>
>>>>
>>>> Perhaps we should just keep __asan_load* as is and leave the decision
>>>> whether to abort or continue for the runtime?  This would make semantics
>>>> of
>>>> -fsanitize-recover cumbersome though (because it wouldn't work if user
>>>> selects outline instrumentation).
>>>
>>>
>>> Well, the "don't ever report anything while some per-CPU flag is set"
>>> thing
>>> can be considered as part of the "is this memory access ok" test, it is
>>> pretending everything is accessible.
>>>
>>> But, otherwise, if it is supposed to be developer's decision at compile
>>> time, __asan_load*_noabort should better always continue, even if it
>>> reported issues, and __asan_load* should better not return after
>>> reporting
>>> errors.
>>>
>>
>> True, but why we need new functions for that.
>> __asan_load could also abort or not depending on what user/developer
>> wants.
>> Why we have to rebuild the entire kernel if someone wants to switch from
>> abort to noabort?
>>
>> I'm not against __asan_load_noabort, I'm just saying that this is no point
>> to have separate
>> __asan_load/__asan_load_noabort functions in kernel.
>
>
> I'd still suggest to emit __asan_load_noabort so that we match userspace
> (where __asan_load strictly matches __asan_report in terminating the
> program).  Behavior of __asan_load_noabort can further be restricted by user
> via various environment settings (kernel parameters, /proc, etc.).
>
> @Dmitry: what's your opinion on this?


I am somewhat lost in this thread and probably missing something.
But why do we need __asan_load (which is not noabort) at all? Outline
instrumentation is non a default mode for both user-space asan and
kasan (at least in the envisioned future). I would expect that these
non-typical cases that use outline instrumentation can also bear the
overhead of non-noreturn functions. Can we use just one version of
__asan_load and let runtime decide on abort?