Re: [PATCH v5 2/6] kasan: Add KASAN mode kernel parameter

[Vincenzo Frascino]

On 1/21/21 5:34 PM, Andrey Konovalov wrote:
>> +- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
>> +  synchronous or asynchronous mode of execution (default: ``sync``).
>> +  ``synchronous mode``: an exception is triggered if a tag check fault occurs.
> Synchronous mode: a bad access is detected immediately when a tag
> check fault occurs.
> 
> (No need for `` here, "synchronous mode" is not an inline snippet.)
> 

Ok will do in v5.

>> +  ``asynchronous mode``: if a tag check fault occurs, the information is stored
>> +  asynchronously in hardware (e.g. in the TFSR_EL1 register for arm64). The kernel
>> +  checks the hardware location and reports an error if the fault is detected.
> Asynchronous mode: a bad access detection is delayed. When a tag check
> fault occurs, the information is stored in hardware (in the TFSR_EL1
> register for arm64). The kernel periodically checks the hardware and
> only reports tag faults during these checks.
> 

Will do in v5.

>> +
>>  - ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
>>    traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
>>    ``off``).
>> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
>> index d16ec9e66806..7285dcf9fcc1 100644
>> --- a/lib/test_kasan.c
>> +++ b/lib/test_kasan.c
>> @@ -97,7 +97,7 @@ static void kasan_test_exit(struct kunit *test)
>>                         READ_ONCE(fail_data.report_found));     \
>>         if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) {                 \
>>                 if (READ_ONCE(fail_data.report_found))          \
>> -                       hw_enable_tagging();                    \
>> +                       hw_enable_tagging_sync();               \
>>                 migrate_enable();                               \
>>         }                                                       \
>>  } while (0)
>> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
>> index e529428e7a11..224a2187839c 100644
>> --- a/mm/kasan/hw_tags.c
>> +++ b/mm/kasan/hw_tags.c
>> @@ -25,6 +25,11 @@ enum kasan_arg {
>>         KASAN_ARG_ON,
>>  };
>>
>> +enum kasan_arg_mode {
>> +       KASAN_ARG_MODE_SYNC,
>> +       KASAN_ARG_MODE_ASYNC,
> For other modes I explicitly added a _DEFAULT option first. It makes
> sense to do this here as well for consistency.
> 

Will do in v5.

>> +};
>> +
>>  enum kasan_arg_stacktrace {
>>         KASAN_ARG_STACKTRACE_DEFAULT,
>>         KASAN_ARG_STACKTRACE_OFF,
>> @@ -38,6 +43,7 @@ enum kasan_arg_fault {
>>  };
>>
>>  static enum kasan_arg kasan_arg __ro_after_init;
>> +static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
>>  static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init;
>>  static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
>>
>> @@ -68,6 +74,21 @@ static int __init early_kasan_flag(char *arg)
>>  }
>>  early_param("kasan", early_kasan_flag);
>>
>> +/* kasan.mode=sync/async */
>> +static int __init early_kasan_mode(char *arg)
>> +{
>> +       /* If arg is not set the default mode is sync */
>> +       if ((!arg) || !strcmp(arg, "sync"))
>> +               kasan_arg_mode = KASAN_ARG_MODE_SYNC;
>> +       else if (!strcmp(arg, "async"))
>> +               kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
>> +       else
>> +               return -EINVAL;
>> +
>> +       return 0;
>> +}
>> +early_param("kasan.mode", early_kasan_mode);
>> +
>>  /* kasan.stacktrace=off/on */
>>  static int __init early_kasan_flag_stacktrace(char *arg)
>>  {
>> @@ -115,7 +136,11 @@ void kasan_init_hw_tags_cpu(void)
>>                 return;
>>
>>         hw_init_tags(KASAN_TAG_MAX);
>> -       hw_enable_tagging();
>> +
> Let's add a comment:
> 
> /* Enable async mode only when explicitly requested through the command line. */
> 

Will do in v5.

-- 
Regards,
Vincenzo