[PATCH v6 17/18] x86/kasan: Logical bit shift for kasan_mem_to_shadow

Maciej Wieczor-Retman posted 18 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v6 17/18] x86/kasan: Logical bit shift for kasan_mem_to_shadow
Posted by Maciej Wieczor-Retman 1 month, 2 weeks ago
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

While generally tag-based KASAN adopts an arithemitc bit shift to
convert a memory address to a shadow memory address, it doesn't work for
all cases on x86. Testing different shadow memory offsets proved that
either 4 or 5 level paging didn't work correctly or inline mode ran into
issues. Thus the best working scheme is the logical bit shift and
non-canonical shadow offset that x86 uses for generic KASAN, of course
adjusted for the increased granularity from 8 to 16 bytes.

Add an arch specific implementation of kasan_mem_to_shadow() that uses
the logical bit shift.

The non-canonical hook tries to calculate whether an address came from
kasan_mem_to_shadow(). First it checks whether this address fits into
the legal set of values possible to output from the mem to shadow
function.

Tie both generic and tag-based x86 KASAN modes to the address range
check associated with generic KASAN.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v4:
- Add this patch to the series.

 arch/x86/include/asm/kasan.h | 7 +++++++
 mm/kasan/report.c            | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
index 375651d9b114..2372397bc3e5 100644
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -49,6 +49,13 @@
 #include <linux/bits.h>
 
 #ifdef CONFIG_KASAN_SW_TAGS
+static inline void *__kasan_mem_to_shadow(const void *addr)
+{
+	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
+		+ KASAN_SHADOW_OFFSET;
+}
+
+#define kasan_mem_to_shadow(addr)	__kasan_mem_to_shadow(addr)
 #define __tag_shifted(tag)		FIELD_PREP(GENMASK_ULL(60, 57), tag)
 #define __tag_reset(addr)		(sign_extend64((u64)(addr), 56))
 #define __tag_get(addr)			((u8)FIELD_GET(GENMASK_ULL(60, 57), (u64)addr))
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 50d487a0687a..fd8fe004b0c0 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -642,13 +642,14 @@ void kasan_non_canonical_hook(unsigned long addr)
 	const char *bug_type;
 
 	/*
-	 * For Generic KASAN, kasan_mem_to_shadow() uses the logical right shift
+	 * For Generic KASAN and Software Tag-Based mode on the x86
+	 * architecture, kasan_mem_to_shadow() uses the logical right shift
 	 * and never overflows with the chosen KASAN_SHADOW_OFFSET values (on
 	 * both x86 and arm64). Thus, the possible shadow addresses (even for
 	 * bogus pointers) belong to a single contiguous region that is the
 	 * result of kasan_mem_to_shadow() applied to the whole address space.
 	 */
-	if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC) || IS_ENABLED(CONFIG_X86_64)) {
 		if (addr < (unsigned long)kasan_mem_to_shadow((void *)(0UL)) ||
 		    addr > (unsigned long)kasan_mem_to_shadow((void *)(~0UL)))
 			return;
-- 
2.51.0
Re: [PATCH v6 17/18] x86/kasan: Logical bit shift for kasan_mem_to_shadow
Posted by Marco Elver 1 month ago
On Wed, 29 Oct 2025 at 21:11, Maciej Wieczor-Retman
<m.wieczorretman@pm.me> wrote:
>
> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>
> While generally tag-based KASAN adopts an arithemitc bit shift to
> convert a memory address to a shadow memory address, it doesn't work for
> all cases on x86. Testing different shadow memory offsets proved that
> either 4 or 5 level paging didn't work correctly or inline mode ran into
> issues. Thus the best working scheme is the logical bit shift and
> non-canonical shadow offset that x86 uses for generic KASAN, of course
> adjusted for the increased granularity from 8 to 16 bytes.
>
> Add an arch specific implementation of kasan_mem_to_shadow() that uses
> the logical bit shift.
>
> The non-canonical hook tries to calculate whether an address came from
> kasan_mem_to_shadow(). First it checks whether this address fits into
> the legal set of values possible to output from the mem to shadow
> function.
>
> Tie both generic and tag-based x86 KASAN modes to the address range
> check associated with generic KASAN.
>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v4:
> - Add this patch to the series.
>
>  arch/x86/include/asm/kasan.h | 7 +++++++
>  mm/kasan/report.c            | 5 +++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> index 375651d9b114..2372397bc3e5 100644
> --- a/arch/x86/include/asm/kasan.h
> +++ b/arch/x86/include/asm/kasan.h
> @@ -49,6 +49,13 @@
>  #include <linux/bits.h>
>
>  #ifdef CONFIG_KASAN_SW_TAGS
> +static inline void *__kasan_mem_to_shadow(const void *addr)
> +{
> +       return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
> +               + KASAN_SHADOW_OFFSET;
> +}

You're effectively undoing "kasan: sw_tags: Use arithmetic shift for
shadow computation" for x86 - why?
This function needs a comment explaining this.

Also, the commit message just says "it doesn't work for all cases" - why?
Re: [PATCH v6 17/18] x86/kasan: Logical bit shift for kasan_mem_to_shadow
Posted by Maciej Wieczór-Retman 3 weeks, 5 days ago
On 2025-11-10 at 15:49:22 +0100, Marco Elver wrote:
>On Wed, 29 Oct 2025 at 21:11, Maciej Wieczor-Retman
><m.wieczorretman@pm.me> wrote:
>>
>> From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>>
>> While generally tag-based KASAN adopts an arithemitc bit shift to
>> convert a memory address to a shadow memory address, it doesn't work for
>> all cases on x86. Testing different shadow memory offsets proved that
>> either 4 or 5 level paging didn't work correctly or inline mode ran into
>> issues. Thus the best working scheme is the logical bit shift and
>> non-canonical shadow offset that x86 uses for generic KASAN, of course
>> adjusted for the increased granularity from 8 to 16 bytes.
>>
>> Add an arch specific implementation of kasan_mem_to_shadow() that uses
>> the logical bit shift.
>>
>> The non-canonical hook tries to calculate whether an address came from
>> kasan_mem_to_shadow(). First it checks whether this address fits into
>> the legal set of values possible to output from the mem to shadow
>> function.
>>
>> Tie both generic and tag-based x86 KASAN modes to the address range
>> check associated with generic KASAN.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v4:
>> - Add this patch to the series.
>>
>>  arch/x86/include/asm/kasan.h | 7 +++++++
>>  mm/kasan/report.c            | 5 +++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
>> index 375651d9b114..2372397bc3e5 100644
>> --- a/arch/x86/include/asm/kasan.h
>> +++ b/arch/x86/include/asm/kasan.h
>> @@ -49,6 +49,13 @@
>>  #include <linux/bits.h>
>>
>>  #ifdef CONFIG_KASAN_SW_TAGS
>> +static inline void *__kasan_mem_to_shadow(const void *addr)
>> +{
>> +       return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>> +               + KASAN_SHADOW_OFFSET;
>> +}
>
>You're effectively undoing "kasan: sw_tags: Use arithmetic shift for
>shadow computation" for x86 - why?
>This function needs a comment explaining this.

Sure, I'll add a comment here.

While the signed approach seems to work well for arm64 and risc-v it
doesn't play well with x86 which wants to keep the top bit for
canonicality checks.

Trying to keep signed mem to shadow scheme for all corner cases on all
configs always turned into ugly workarounds for something. There is a
mechanism, when there is a fault, to guess if the address came from a
KASAN check - some address format always didn't work when I tried
validating 4 and 5 paging levels. One approach to getting the signed mem
to shadow was also using a non-canonial kasan shadow offset. It worked
great for paging as far as I remember (some 5 lvl fixup code could be
removed) but it made the inline mode either hard to implement or much
slower due to extended checks.

>Also, the commit message just says "it doesn't work for all cases" - why?

Fair enough, this was a little not verbose. I'll update the patch
message with an explanation.