On Thu, Feb 15, 2024 at 12:25 AM Benjamin Gray <bgray@xxxxxxxxxxxxx> wrote: > > > We can also add something like CONFIG_ARCH_HAS_KASAN_FLAG_ENABLE and > > only use a static branch only on those architectures where it's > > required. Perhaps CONFIG_ARCH_USES_KASAN_ENABLED would be a better name. > That works too, PowerPC should only need a static branch when > CONFIG_KASAN is enabled. > > Loongarch is also a kasan_arch_is_ready() user though, so I'm not sure > if they'd still need it for something? And UM as well. We can start with a change that makes kasan_flag_enabled and kasan_enabled() work for the Generic mode, define a kasan_init_generic() function that switches kasan_flag_enabled (and prints the "KernelAddressSanitizer initialized" message?), and then call kasan_init_generic() from kasan_init() in the arch code (perhaps even for all arches to unify the "initialized" message?). And then we can ask Loongarch and UM people to test the change. Both Loongarch and UM define kasan_arch_is_ready() as the value of their global enable flags, which they switch in kasan_init(). So I would think using kasan_enabled() should just work (minding the metadata-related parts). > > What was this data access? Is this something we need to fix in the > > mainline? > > I don't believe so (though I spent a while debugging it before I > realised I had introduced it by changing kasan_enabled() dynamically). > > In kasan_cache_create() we unconditionally allocate a metadata buffer, > but the kasan_init_slab_obj() call to initialise it is guarded by > kasan_enabled(). But later parts of the code only check the presence of > the buffer before using it, so bad things happen if kasan_enabled() > later turns on (I was getting some error about invalid lock state). Ah, makes sense. Yeah, these metadata init functions should work even before kasan_flag_enabled is switched on then.
