Hi Andrey,
[...]
> > static inline const char *kasan_mode_info(void)
> > {
> > if (kasan_mode == KASAN_MODE_ASYNC)
> > @@ -257,15 +283,26 @@ void __init kasan_init_hw_tags(void)
> > break;
> > }
> >
> > + switch (kasan_arg_write_only) {
> > + case KASAN_ARG_WRITE_ONLY_DEFAULT:
>
> Let's keep this part similar to the other parameters for consistency:
>
> /* Default is specified by kasan_flag_write_only definition. */
> break;
Okay.
> > + case KASAN_ARG_WRITE_ONLY_OFF:
> > + kasan_flag_write_only = false;
> > + break;
> > + case KASAN_ARG_WRITE_ONLY_ON:
> > + kasan_flag_write_only = true;
> > + break;
> > + }
> > +
> > kasan_init_tags();
> >
> > /* KASAN is now initialized, enable it. */
> > static_branch_enable(&kasan_flag_enabled);
> >
> > - pr_info("KernelAddressSanitizer initialized (hw-tags, mode=%s, vmalloc=%s, stacktrace=%s)\n",
> > + pr_info("KernelAddressSanitizer initialized (hw-tags, mode=%s, vmalloc=%s, stacktrace=%s, write_only=%s\n",
> > kasan_mode_info(),
> > str_on_off(kasan_vmalloc_enabled()),
> > - str_on_off(kasan_stack_collection_enabled()));
> > + str_on_off(kasan_stack_collection_enabled()),
> > + str_on_off(kasan_arg_write_only));
> > }
> >
> > #ifdef CONFIG_KASAN_VMALLOC
> > @@ -392,6 +429,13 @@ void kasan_enable_hw_tags(void)
> > hw_enable_tag_checks_asymm();
> > else
> > hw_enable_tag_checks_sync();
> > +
> > + if (kasan_arg_write_only == KASAN_ARG_WRITE_ONLY_ON &&
>
> We already set kasan_flag_write_only by this point, right? Let's check
> that one then.
Yes. if user specifies the write_only == on.
>
> > + hw_enable_tag_checks_write_only()) {
> > + kasan_arg_write_only == KASAN_ARG_WRITE_ONLY_OFF;
>
> Typo in == in the line above. But also I think we can just drop the
> line: kasan_arg_write_only is KASAN_ARG_WRITE_ONLY_ON after all, it's
> just not supported and thus kasan_flag_write_only is set to false to
> reflect that.
Sorry :\ I've missed this fix from patch 3... this should be == to =.
However, we couldn't remove kasan_arg_write_only check in condition.
If one of cpu get failed to hw_enable_tag_checks_write_only() then
By changing this with KASAN_ARG_WRITE_ONLY_OFF, It prevent to call
hw_eanble_tag_checks_write_only() in other cpu.
As you said, kasan_flag_write_only reflects the state.
But like other option, I keep the condition to call the hw_enable_xxx()
by checking the "argments" and keep the "hw enable state" with
kasan_flag_write_only.
so let me change == to = only in here.
>
> > + kasan_flag_write_only = false;
> > + pr_warn_once("System doesn't support write-only option. Disable it\n");
>
> Let's do pr_err like the rest of KASAN code. And:
>
> pr_err_once("write-only mode is not supported and thus not enabled\n");
Okay. Thanks for suggestion.
>
>
>
> > + }
> > }
> >
> > #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
> > @@ -404,4 +448,10 @@ VISIBLE_IF_KUNIT void kasan_force_async_fault(void)
> > }
> > EXPORT_SYMBOL_IF_KUNIT(kasan_force_async_fault);
> >
> > +VISIBLE_IF_KUNIT bool kasan_write_only_enabled(void)
> > +{
> > + return kasan_flag_write_only;
> > +}
> > +EXPORT_SYMBOL_IF_KUNIT(kasan_write_only_enabled);
> > +
> > #endif
> > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> > index 129178be5e64..c1490136c96b 100644
> > --- a/mm/kasan/kasan.h
> > +++ b/mm/kasan/kasan.h
> > @@ -428,6 +428,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
> > #define hw_enable_tag_checks_sync() arch_enable_tag_checks_sync()
> > #define hw_enable_tag_checks_async() arch_enable_tag_checks_async()
> > #define hw_enable_tag_checks_asymm() arch_enable_tag_checks_asymm()
> > +#define hw_enable_tag_checks_write_only() arch_enable_tag_checks_write_only()
> > #define hw_suppress_tag_checks_start() arch_suppress_tag_checks_start()
> > #define hw_suppress_tag_checks_stop() arch_suppress_tag_checks_stop()
> > #define hw_force_async_tag_fault() arch_force_async_tag_fault()
> > @@ -437,11 +438,17 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
> > arch_set_mem_tag_range((addr), (size), (tag), (init))
> >
> > void kasan_enable_hw_tags(void);
> > +bool kasan_write_only_enabled(void);
>
> This should go next to kasan_force_async_fault().
>
> >
> > #else /* CONFIG_KASAN_HW_TAGS */
> >
> > static inline void kasan_enable_hw_tags(void) { }
> >
> > +static inline bool kasan_write_only_enabled(void)
> > +{
> > + return false;
> > +}
>
> And this too.
Right. I'll move them. Thanks!
Thanks!
--
Sincerely,
Yeoreum Yun
