Re: [PATCH] kasan: also display registers for reports from HW exceptions

[Andrey Konovalov <andreyknvl@gmail.com>]

On Sat, Sep 10, 2022 at 7:24 AM Peter Collingbourne <pcc@google.com> wrote:
>
> It is sometimes useful to know the values of the registers when a KASAN
> report is generated.

Hi Peter,

What are the cases when the register values are useful? They are
"corrupted" by KASAN runtime anyway and thus are not relevant to the
place in code where the bad access happened.

Thanks!

> We can do this easily for reports that resulted from
> a hardware exception by passing the struct pt_regs from the exception into
> the report function; do so.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> ---
> Applies to -next.
>
>  arch/arm64/kernel/traps.c |  3 +--
>  arch/arm64/mm/fault.c     |  2 +-
>  include/linux/kasan.h     | 10 ++++++++++
>  mm/kasan/kasan.h          |  1 +
>  mm/kasan/report.c         | 27 ++++++++++++++++++++++-----
>  5 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index b7fed33981f7..42f05f38c90a 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -1019,9 +1019,8 @@ static int kasan_handler(struct pt_regs *regs, unsigned long esr)
>         bool write = esr & KASAN_ESR_WRITE;
>         size_t size = KASAN_ESR_SIZE(esr);
>         u64 addr = regs->regs[0];
> -       u64 pc = regs->pc;
>
> -       kasan_report(addr, size, write, pc);
> +       kasan_report_regs(addr, size, write, regs);
>
>         /*
>          * The instrumentation allows to control whether we can proceed after
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 5b391490e045..c4b91f5d8cc8 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -316,7 +316,7 @@ static void report_tag_fault(unsigned long addr, unsigned long esr,
>          * find out access size.
>          */
>         bool is_write = !!(esr & ESR_ELx_WNR);
> -       kasan_report(addr, 0, is_write, regs->pc);
> +       kasan_report_regs(addr, 0, is_write, regs);
>  }
>  #else
>  /* Tag faults aren't enabled without CONFIG_KASAN_HW_TAGS. */
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index d811b3d7d2a1..381aea149353 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -353,6 +353,16 @@ static inline void *kasan_reset_tag(const void *addr)
>  bool kasan_report(unsigned long addr, size_t size,
>                 bool is_write, unsigned long ip);
>
> +/**
> + * kasan_report_regs - print a report about a bad memory access detected by KASAN
> + * @addr: address of the bad access
> + * @size: size of the bad access
> + * @is_write: whether the bad access is a write or a read
> + * @regs: register values at the point of the bad memory access
> + */
> +bool kasan_report_regs(unsigned long addr, size_t size, bool is_write,
> +                      struct pt_regs *regs);
> +
>  #else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>
>  static inline void *kasan_reset_tag(const void *addr)
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index abbcc1b0eec5..39772c21a8ae 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -175,6 +175,7 @@ struct kasan_report_info {
>         size_t access_size;
>         bool is_write;
>         unsigned long ip;
> +       struct pt_regs *regs;
>
>         /* Filled in by the common reporting code. */
>         void *first_bad_addr;
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 39e8e5a80b82..eac9cd45b4a1 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -24,6 +24,7 @@
>  #include <linux/types.h>
>  #include <linux/kasan.h>
>  #include <linux/module.h>
> +#include <linux/sched/debug.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/uaccess.h>
>  #include <trace/events/error_report.h>
> @@ -284,7 +285,6 @@ static void print_address_description(void *addr, u8 tag,
>  {
>         struct page *page = addr_to_page(addr);
>
> -       dump_stack_lvl(KERN_ERR);
>         pr_err("\n");
>
>         if (info->cache && info->object) {
> @@ -394,11 +394,14 @@ static void print_report(struct kasan_report_info *info)
>                 kasan_print_tags(tag, info->first_bad_addr);
>         pr_err("\n");
>
> +       if (info->regs)
> +               show_regs(info->regs);
> +       else
> +               dump_stack_lvl(KERN_ERR);
> +
>         if (addr_has_metadata(addr)) {
>                 print_address_description(addr, tag, info);
>                 print_memory_metadata(info->first_bad_addr);
> -       } else {
> -               dump_stack_lvl(KERN_ERR);
>         }
>  }
>
> @@ -458,8 +461,8 @@ void kasan_report_invalid_free(void *ptr, unsigned long ip, enum kasan_report_ty
>   * user_access_save/restore(): kasan_report_invalid_free() cannot be called
>   * from a UACCESS region, and kasan_report_async() is not used on x86.
>   */
> -bool kasan_report(unsigned long addr, size_t size, bool is_write,
> -                       unsigned long ip)
> +static bool __kasan_report(unsigned long addr, size_t size, bool is_write,
> +                       unsigned long ip, struct pt_regs *regs)
>  {
>         bool ret = true;
>         void *ptr = (void *)addr;
> @@ -480,6 +483,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
>         info.access_size = size;
>         info.is_write = is_write;
>         info.ip = ip;
> +       info.regs = regs;
>
>         complete_report_info(&info);
>
> @@ -493,6 +497,19 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
>         return ret;
>  }
>
> +bool kasan_report(unsigned long addr, size_t size, bool is_write,
> +                       unsigned long ip)
> +{
> +       return __kasan_report(addr, size, is_write, ip, NULL);
> +}
> +
> +bool kasan_report_regs(unsigned long addr, size_t size, bool is_write,
> +                      struct pt_regs *regs)
> +{
> +       return __kasan_report(addr, size, is_write, instruction_pointer(regs),
> +                             regs);
> +}
> +
>  #ifdef CONFIG_KASAN_HW_TAGS
>  void kasan_report_async(void)
>  {
> --
> 2.37.2.789.g6183377224-goog
>