Skip to content

Commit

Permalink
UPSTREAM: kasan: suppress recursive reports for HW_TAGS
Browse files Browse the repository at this point in the history
KASAN suppresses reports for bad accesses done by the KASAN reporting
code.  The reporting code might access poisoned memory for reporting
purposes.

Software KASAN modes do this by suppressing reports during reporting via
current->kasan_depth, the same way they suppress reports during accesses
to poisoned slab metadata.

Hardware Tag-Based KASAN does not use current->kasan_depth, and instead
resets pointer tags for accesses to poisoned memory done by the reporting
code.

Despite that, a recursive report can still happen:

1. On hardware with faulty MTE support. This was observed by Weizhao
   Ouyang on a faulty hardware that caused memory tags to randomly change
   from time to time.

2. Theoretically, due to a previous MTE-undetected memory corruption.

A recursive report can happen via:

1. Accessing a pointer with a non-reset tag in the reporting code, e.g.
   slab->slab_cache, which is what Weizhao Ouyang observed.

2. Theoretically, via external non-annotated routines, e.g. stackdepot.

To resolve this issue, resetting tags for all of the pointers in the
reporting code and all the used external routines would be impractical.

Instead, disable tag checking done by the CPU for the duration of KASAN
reporting for Hardware Tag-Based KASAN.

Without this fix, Hardware Tag-Based KASAN reporting code might deadlock.

[[email protected]: disable preemption instead of migration, fix comment typo]
  Link: https://lkml.kernel.org/r/d14417c8bc5eea7589e99381203432f15c0f9138.1680114854.git.andreyknvl@google.com
Link: https://lkml.kernel.org/r/59f433e00f7fa985e8bf9f7caf78574db16b67ab.1678491668.git.andreyknvl@google.com
Fixes: 2e903b9 ("kasan, arm64: implement HW_TAGS runtime")
Signed-off-by: Andrey Konovalov <[email protected]>
Reported-by: Weizhao Ouyang <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Evgenii Stepanov <[email protected]>
Cc: Peter Collingbourne <[email protected]>
Cc: Vincenzo Frascino <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Bug: 254721825
(cherry picked from commit c6a690e0c978bda8106e7a489c13323f90b087d0)
Signed-off-by: Andrey Konovalov <[email protected]>
Change-Id: Ifc5daf66f57dd16e85de73257cc0966565836269
  • Loading branch information
xairy authored and Treehugger Robot committed Jul 12, 2023
1 parent c0226bf commit 7c2b6c7
Showing 1 changed file with 48 additions and 11 deletions.
59 changes: 48 additions & 11 deletions mm/kasan/report.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,18 @@ static int __init kasan_set_multi_shot(char *str)
__setup("kasan_multi_shot", kasan_set_multi_shot);

/*
* Used to suppress reports within kasan_disable/enable_current() critical
* sections, which are used for marking accesses to slab metadata.
* This function is used to check whether KASAN reports are suppressed for
* software KASAN modes via kasan_disable/enable_current() critical sections.
*
* This is done to avoid:
* 1. False-positive reports when accessing slab metadata,
* 2. Deadlocking when poisoned memory is accessed by the reporting code.
*
* Hardware Tag-Based KASAN instead relies on:
* For #1: Resetting tags via kasan_reset_tag().
* For #2: Suppression of tag checks via CPU, see report_suppress_start/end().
*/
static bool report_suppressed(void)
static bool report_suppressed_sw(void)
{
#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
if (current->kasan_depth)
Expand All @@ -85,6 +93,30 @@ static bool report_suppressed(void)
return false;
}

static void report_suppress_start(void)
{
#ifdef CONFIG_KASAN_HW_TAGS
/*
* Disable preemption for the duration of printing a KASAN report, as
* hw_suppress_tag_checks_start() disables checks on the current CPU.
*/
preempt_disable();
hw_suppress_tag_checks_start();
#else
kasan_disable_current();
#endif
}

static void report_suppress_stop(void)
{
#ifdef CONFIG_KASAN_HW_TAGS
hw_suppress_tag_checks_stop();
preempt_enable();
#else
kasan_enable_current();
#endif
}

/*
* Used to avoid reporting more than one KASAN bug unless kasan_multi_shot
* is enabled. Note that KASAN tests effectively enable kasan_multi_shot
Expand Down Expand Up @@ -152,7 +184,7 @@ static void start_report(unsigned long *flags, bool sync)
/* Do not allow LOCKDEP mangling KASAN reports. */
lockdep_off();
/* Make sure we don't end up in loop. */
kasan_disable_current();
report_suppress_start();
spin_lock_irqsave(&report_lock, *flags);
pr_err("==================================================================\n");
}
Expand All @@ -170,7 +202,7 @@ static void end_report(unsigned long *flags, void *addr)
panic("kasan.fault=panic set ...\n");
add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
lockdep_on();
kasan_enable_current();
report_suppress_stop();
}

static void print_error_description(struct kasan_report_info *info)
Expand Down Expand Up @@ -439,9 +471,13 @@ void kasan_report_invalid_free(void *ptr, unsigned long ip, enum kasan_report_ty
struct kasan_report_info info;

/*
* Do not check report_suppressed(), as an invalid-free cannot be
* caused by accessing slab metadata and thus should not be
* suppressed by kasan_disable/enable_current() critical sections.
* Do not check report_suppressed_sw(), as an invalid-free cannot be
* caused by accessing poisoned memory and thus should not be suppressed
* by kasan_disable/enable_current() critical sections.
*
* Note that for Hardware Tag-Based KASAN, kasan_report_invalid_free()
* is triggered by explicit tag checks and not by the ones performed by
* the CPU. Thus, reporting invalid-free is not suppressed as well.
*/
if (unlikely(!report_enabled()))
return;
Expand Down Expand Up @@ -476,7 +512,7 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
unsigned long irq_flags;
struct kasan_report_info info;

if (unlikely(report_suppressed()) || unlikely(!report_enabled())) {
if (unlikely(report_suppressed_sw()) || unlikely(!report_enabled())) {
ret = false;
goto out;
}
Expand Down Expand Up @@ -508,8 +544,9 @@ void kasan_report_async(void)
unsigned long flags;

/*
* Do not check report_suppressed(), as kasan_disable/enable_current()
* critical sections do not affect Hardware Tag-Based KASAN.
* Do not check report_suppressed_sw(), as
* kasan_disable/enable_current() critical sections do not affect
* Hardware Tag-Based KASAN.
*/
if (unlikely(!report_enabled()))
return;
Expand Down

0 comments on commit 7c2b6c7

Please sign in to comment.