On Thu, Sep 4, 2025 at 10:11 AM Baoquan He <b...@redhat.com> wrote: > > > If so, would it help if we make the kasan.vmalloc command-line > > parameter work with the non-HW_TAGS modes (and make it do the same > > thing as disabling CONFIG_KASAN_VMALLOC)? > > > > What I don't like about introducing kasan=off for non-HW_TAGS modes is > > that this parameter does not actually disable KASAN. It just > > suppresses KASAN code for mapping proper shadow memory. But the > > compiler-added instrumentation is still executing (and I suspect this > > might break the inline instrumentation mode). > > I may not follow your saying it doesn't disable KASAN. In this patchset, > not only do I disable the code for mapping shadow memory, but also I > skip any KASAN checking. Please see change of check_region_inline() in > mm/kasan/generic.c and kasan_check_range() in mm/kasan/sw_tags.c. It > will skip any KASAN checking when accessing memory. > > Yeah, the compiler added instrumentation will be called, but the if > (!kasan_enabled()) checking will decide if going further into KASAN code > or just return directly.
This all is true for the outline instrumentation mode. However, with the inline instrumentation, check_region_inline() is not called (in many cases, at least) and instead the compiler embeds the instructions to calculate the shadow memory address and check its value directly (this is why we have CONFIG_KASAN_SHADOW_OFFSET, whose value has to be known at compile time). > I tried inline mode on x86_64 and arm64, it > works well when one reviewer said inline mode could cost much more > memory, I don't see any breakage w or w/o kasan=off when this patchset > applied.. This is interesting. I guess what happens is that we still have the early shadow memory mapped so the shadow memory accesses inserted by the inline instrumentation do not crash. But have you tried running kasan=off + CONFIG_KASAN_STACK=y + CONFIG_VMAP_STACK=y (+ CONFIG_KASAN_VMALLOC=y)? I would expect this should causes crashes, as the early shadow is mapped as read-only and the inline stack instrumentation will try writing into it (or do the writes into the early shadow somehow get ignored?..). > > Perhaps, we could instead add a new kasan.shadow=on/off parameter to > > make it more explicit that KASAN is not off, it's just that it stops > > mapping shadow memory. > > Hmm, as I explained at above, kasan=off will stop mapping shadow memory, > and also stop executing KASAN code to poison/unpoison memory and check the > shadow. It may be inappropriate to say it only stops mapping shadow. That's true, but we can only achieve this for the outline instrumentation mode. With the inline instrumentation mode, the (early) shadow memory would still get accessed all the time even with kasan=off. Which can be considered inappropriate, as you pointed out (though this is what happens for vmalloc allocations when CONFIG_KASAN_VMALLOC is disabled and it does seem to work; but the inline stack instrumentation might be a problem). We could limit kasan=off to only the outline instrumentation mode, but I guess that defeats the purpose. I'm not completely opposed to making kasan=off work with all KASAN modes (assuming it works with the inline instrumentation), but then we will need to thoroughly document the behavior it creates. And let's also wait for an opinion from the other KASAN maintainers on this. > > Dmitry, Alexander, Marco, do you have any opinion on kasan=off for > > non-HW_TAGS modes? > > > > On a side note, this series will need to be rebased onto Sabyrzhan's > > patches [1] - those are close to being ready. But perhaps let's wait > > for v7 first. > > I replied to Sabyrzhan's patchset, on top of this patchset, it's much > easier and cleaner to remove kasan_arch_is_ready(). We don't need > introduce CONFIG_ARCH_DEFER_KASAN. Please see below patchset which is > based on this patchset introducing 'kasan=off|on' to genric|sw_tags > mode. Based on a brief look, both patch series seem to be doing similar things (except yours also allows using kasan=off for all modes). But I like the Sabyrzhan's approach of hiding the explicit static_branch_enable() calls under CONFIG_ARCH_DEFER_KASAN for the architectures where they are actually required. So I propose we moved forward with the Sabyrzhan's series and then apply additional patches for supporting kasan=off on top (but again, assuming they work with the inline instrumentation). Thank you!
