On Fri, Oct 24, 2014 at 12:28 PM, Yury Gribov <y.gri...@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?