[PATCH v2 00/14] kasan: x86: arm64: KASAN tag-based mode for x86

[PATCH v2 00/14] kasan: x86: arm64: KASAN tag-based mode for x86

[Maciej Wieczor-Retman]

Changes v2:
- Split the series into one adding KASAN tag-based mode (this one) and
  another one that adds the dense mode to KASAN (will post later).
- Removed exporting kasan_poison() and used a wrapper instead in
  kasan_init_64.c
- Prepended series with 4 patches from the risc-v series and applied
  review comments to the first patch as the rest already are reviewed.

======= Introduction
The patchset aims to add a KASAN tag-based mode for the x86 architecture
with the help of the new CPU feature called Linear Address Masking
(LAM). Main improvement introduced by the series is 2x lower memory
usage compared to KASAN's generic mode, the only currently available
mode on x86.

There are two relevant series in the process of adding KASAN tag-based
support to x86. This one focuses on implementing and enabling the
tag-based mode for the x86 architecture by using LAM. The second one
attempts to add a new memory saving mechanism called "dense mode" to the
non-arch part of the tag-based KASAN code. It can provide another 2x
memory savings by packing tags denser in the shadow memory.

======= How KASAN tag-based mode works?
When enabled, memory accesses and allocations are augmented by the
compiler during kernel compilation. Instrumentation functions are added
to each memory allocation and each pointer dereference.

The allocation related functions generate a random tag and save it in
two places: in shadow memory that maps to the allocated memory, and in
the top bits of the pointer that points to the allocated memory. Storing
the tag in the top of the pointer is possible because of Top-Byte Ignore
(TBI) on arm64 architecture and LAM on x86.

The access related functions are performing a comparison between the tag
stored in the pointer and the one stored in shadow memory. If the tags
don't match an out of bounds error must have occurred and so an error
report is generated.

The general idea for the tag-based mode is very well explained in the
series with the original implementation [1].

[1] https://lore.kernel.org/all/cover.1544099024.git.andreyknvl@google.com/

======= Differences summary compared to the arm64 tag-based mode
- Tag width:
	- Tag width influences the chance of a tag mismatch due to two
	  tags from different allocations having the same value. The
	  bigger the possible range of tag values the lower the chance
	  of that happening.
	- Shortening the tag width from 8 bits to 4, while it can help
	  with memory usage, it also increases the chance of not
	  reporting an error. 4 bit tags have a ~7% chance of a tag
	  mismatch.

- TBI and LAM
	- TBI in arm64 allows for storing metadata in the top 8 bits of
	  the virtual address.
	- LAM in x86 allows storing tags in bits [62:57] of the pointer.
	  To maximize memory savings the tag width is reduced to bits
	  [60:57].

- KASAN offset calculations
	- When converting addresses from memory to shadow memory the
	  address is treated as a signed number.
	- On arm64 due to TBI half of tagged addresses will be positive
	  and half negative. KASAN shadow offset means the middle point
	  of the shadow memory there.
	- On x86 - due to the top bit of the pointer always being set in
	  kernel address space - all the addresses will be negative when
	  treated as signed offsets into shadow memory. KASAN shadow
	  offset means the end of the shadow memory there.

======= Testing
Checked all the kunits for both software tags and generic KASAN after
making changes.

In generic mode the results were:

kasan: pass:59 fail:0 skip:13 total:72
Totals: pass:59 fail:0 skip:13 total:72
ok 1 kasan

and for software tags:

kasan: pass:63 fail:0 skip:9 total:72
Totals: pass:63 fail:0 skip:9 total:72
ok 1 kasan

======= Benchmarks
All tests were ran on a Sierra Forest server platform with 512GB of
memory. The only differences between the tests were kernel options:
	- CONFIG_KASAN
	- CONFIG_KASAN_GENERIC
	- CONFIG_KASAN_SW_TAGS
	- CONFIG_KASAN_INLINE [1]
	- CONFIG_KASAN_OUTLINE [1]

More benchmarks are noted in the second series that adds the dense mode
to KASAN. That's because most values on x86' tag-based mode are tailored
to work well with that.

Boot time (until login prompt):
* 03:48 for clean kernel
* 08:02 / 09:45 for generic KASAN (inline/outline)
* 08:50 for tag-based KASAN
* 04:50 for tag-based KASAN with stacktrace disabled [2]

Compilation time comparison (10 cores):
* 7:27 for clean kernel
* 8:21/7:44 for generic KASAN (inline/outline)
* 7:41 for tag-based KASAN

[1] Based on hwasan and asan compiler parameters used in
scripts/Makefile.kasan it looks like inline/outline modes have a bigger
impact on generic mode than the tag-based mode. In the former inlining
actually increases the kernel image size and improves performance. In
the latter it un-inlines some code portions for debugging purposes when
the outline mode is chosen but no real difference is visible in
performance and kernel image size.

[2] Memory allocation and freeing performance suffers heavily from saving
stacktraces that can be later displayed in error reports.

======= Compilation
Clang was used to compile the series (make LLVM=1) since gcc doesn't
seem to have support for KASAN tag-based compiler instrumentation on
x86.

======= Dependencies
Series bases on four patches taken from [1] series. Mainly the idea of
converting memory addresses to shadow memory addresses by treating them
as signed as opposed to unsigned. I tried applying review comments to
the first patch, while the other three are unchanged.

The base branch for the series is the tip x86/mm branch.

[1] https://lore.kernel.org/all/20241022015913.3524425-1-samuel.holland@sifive.com/

======= Enabling LAM for testing the series without LASS
Since LASS is needed for LAM and it can't be compiled without it I
enabled LAM during testing with the patch below:

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2275,7 +2275,7 @@ config RANDOMIZE_MEMORY_PHYSICAL_PADDING
 config ADDRESS_MASKING
 	bool "Linear Address Masking support"
 	depends on X86_64
-	depends on COMPILE_TEST || !CPU_MITIGATIONS # wait for LASS
+	#depends on COMPILE_TEST || !CPU_MITIGATIONS # wait for LASS
 	help
 	  Linear Address Masking (LAM) modifies the checking that is applied
 	  to 64-bit linear addresses, allowing software to use of the

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2401,9 +2401,10 @@ void __init arch_cpu_finalize_init(void)

 		/*
 		 * Enable this when LAM is gated on LASS support
+		 */
 		if (cpu_feature_enabled(X86_FEATURE_LAM))
 			USER_PTR_MAX = (1ul << 63) - PAGE_SIZE;
-		 */
+
 		runtime_const_init(ptr, USER_PTR_MAX);

 		/*

Maciej Wieczor-Retman (10):
  kasan: arm64: x86: Make special tags arch specific
  x86: Add arch specific kasan functions
  x86: Reset tag for virtual to physical address conversions
  x86: Physical address comparisons in fill_p*d/pte
  mm: Pcpu chunk address tag reset
  x86: KASAN raw shadow memory PTE init
  x86: LAM initialization
  x86: Minimal SLAB alignment
  x86: runtime_const used for KASAN_SHADOW_END
  x86: Make software tag-based kasan available

Samuel Holland (4):
  kasan: sw_tags: Use arithmetic shift for shadow computation
  kasan: sw_tags: Check kasan_flag_enabled at runtime
  kasan: sw_tags: Support outline stack tag generation
  kasan: sw_tags: Support tag widths less than 8 bits

 Documentation/arch/x86/x86_64/mm.rst |  6 ++--
 MAINTAINERS                          |  2 +-
 arch/arm64/Kconfig                   | 10 +++---
 arch/arm64/include/asm/kasan-tags.h  |  9 +++++
 arch/arm64/include/asm/kasan.h       |  6 ++--
 arch/arm64/include/asm/memory.h      | 14 +++++++-
 arch/arm64/include/asm/uaccess.h     |  1 +
 arch/arm64/mm/kasan_init.c           |  7 ++--
 arch/x86/Kconfig                     |  9 +++--
 arch/x86/boot/compressed/misc.h      |  1 +
 arch/x86/include/asm/kasan-tags.h    |  9 +++++
 arch/x86/include/asm/kasan.h         | 50 +++++++++++++++++++++++++---
 arch/x86/include/asm/page.h          | 17 +++++++---
 arch/x86/include/asm/page_64.h       |  2 +-
 arch/x86/kernel/head_64.S            |  3 ++
 arch/x86/kernel/setup.c              |  2 ++
 arch/x86/kernel/vmlinux.lds.S        |  1 +
 arch/x86/mm/init.c                   |  3 ++
 arch/x86/mm/init_64.c                | 11 +++---
 arch/x86/mm/kasan_init_64.c          | 24 ++++++++++---
 arch/x86/mm/physaddr.c               |  1 +
 include/linux/kasan-enabled.h        | 15 +++------
 include/linux/kasan-tags.h           | 19 ++++++++---
 include/linux/kasan.h                | 14 ++++++--
 include/linux/mm.h                   |  6 ++--
 include/linux/page-flags-layout.h    |  7 +---
 mm/kasan/hw_tags.c                   | 10 ------
 mm/kasan/kasan.h                     |  2 ++
 mm/kasan/report.c                    | 26 ++++++++++++---
 mm/kasan/sw_tags.c                   |  9 +++++
 mm/kasan/tags.c                      | 10 ++++++
 mm/percpu-vm.c                       |  2 +-
 scripts/gdb/linux/mm.py              |  5 +--
 33 files changed, 237 insertions(+), 76 deletions(-)
 create mode 100644 arch/arm64/include/asm/kasan-tags.h
 create mode 100644 arch/x86/include/asm/kasan-tags.h

-- 
2.47.1

[PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

From: Samuel Holland <samuel.holland@sifive.com>

Currently, kasan_mem_to_shadow() uses a logical right shift, which turns
canonical kernel addresses into non-canonical addresses by clearing the
high KASAN_SHADOW_SCALE_SHIFT bits. The value of KASAN_SHADOW_OFFSET is
then chosen so that the addition results in a canonical address for the
shadow memory.

For KASAN_GENERIC, this shift/add combination is ABI with the compiler,
because KASAN_SHADOW_OFFSET is used in compiler-generated inline tag
checks[1], which must only attempt to dereference canonical addresses.

However, for KASAN_SW_TAGS we have some freedom to change the algorithm
without breaking the ABI. Because TBI is enabled for kernel addresses,
the top bits of shadow memory addresses computed during tag checks are
irrelevant, and so likewise are the top bits of KASAN_SHADOW_OFFSET.
This is demonstrated by the fact that LLVM uses a logical right shift
in the tag check fast path[2] but a sbfx (signed bitfield extract)
instruction in the slow path[3] without causing any issues.

Using an arithmetic shift in kasan_mem_to_shadow() provides a number of
benefits:

1) The memory layout is easier to understand. KASAN_SHADOW_OFFSET
becomes a canonical memory address, and the shifted pointer becomes a
negative offset, so KASAN_SHADOW_OFFSET == KASAN_SHADOW_END regardless
of the shift amount or the size of the virtual address space.

2) KASAN_SHADOW_OFFSET becomes a simpler constant, requiring only one
instruction to load instead of two. Since it must be loaded in each
function with a tag check, this decreases kernel text size by 0.5%.

3) This shift and the sign extension from kasan_reset_tag() can be
combined into a single sbfx instruction. When this same algorithm change
is applied to the compiler, it removes an instruction from each inline
tag check, further reducing kernel text size by an additional 4.6%.

These benefits extend to other architectures as well. On RISC-V, where
the baseline ISA does not shifted addition or have an equivalent to the
sbfx instruction, loading KASAN_SHADOW_OFFSET is reduced from 3 to 2
instructions, and kasan_mem_to_shadow(kasan_reset_tag(addr)) similarly
combines two consecutive right shifts.

Due to signed memory-to-shadow mapping kasan_non_canonical_hook() needs
changes - specifically the first part that tries to deduce if a faulty
address came from kasan_mem_to_shadow(). Previous value of
KASAN_SHADOW_OFFSET prevented any overflows when trying to map the
entire linear address space to shadow memory so the check in
kasan_non_canonical_hook() could consist of only checking whether the
address isn't below KASAN_SHADOW_OFFSET.

The signed memory-to-shadow conversion means negative addresses will be
mapped below KASAN_SHADOW_OFFSET and positive addresses will map above
KASAN_SHADOW_OFFSET. When looking at the mapping of the entire address
space there will be an overflow when a big enough positive address will
be passed to kasan_mem_to_shadow(). Then the question of finding
addresses that couldn't come from kasan_mem_to_shadow() can be reduced
to figuring out if the address isn't above the highest overflowed value
(most positive address possible) AND below the most negative address
possible.

Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L1316 [1]
Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L895 [2]
Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp#L669 [3]
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2: (Maciej)
- Correct address range that's checked in kasan_non_canonical_hook().
  Adjust the comment inside.
- Remove part of comment from arch/arm64/include/asm/memory.h.
- Append patch message paragraph about the overflow in
  kasan_non_canonical_hook().

 arch/arm64/Kconfig              | 10 +++++-----
 arch/arm64/include/asm/memory.h | 14 +++++++++++++-
 arch/arm64/mm/kasan_init.c      |  7 +++++--
 include/linux/kasan.h           | 10 ++++++++--
 mm/kasan/report.c               | 26 ++++++++++++++++++++++----
 scripts/gdb/linux/mm.py         |  5 +++--
 6 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fcdd0ed3eca8..fe7d79b447c3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -426,11 +426,11 @@ config KASAN_SHADOW_OFFSET
 	default 0xdffffe0000000000 if ARM64_VA_BITS_42 && !KASAN_SW_TAGS
 	default 0xdfffffc000000000 if ARM64_VA_BITS_39 && !KASAN_SW_TAGS
 	default 0xdffffff800000000 if ARM64_VA_BITS_36 && !KASAN_SW_TAGS
-	default 0xefff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
-	default 0xefffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
-	default 0xeffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
-	default 0xefffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
-	default 0xeffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
+	default 0xffff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
+	default 0xffffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
+	default 0xfffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
+	default 0xffffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
+	default 0xfffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
 	default 0xffffffffffffffff
 
 config UNWIND_TABLES
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 717829df294e..e71cdf036287 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -89,7 +89,15 @@
  *
  * KASAN_SHADOW_END is defined first as the shadow address that corresponds to
  * the upper bound of possible virtual kernel memory addresses UL(1) << 64
- * according to the mapping formula.
+ * according to the mapping formula. For Generic KASAN, the address in the
+ * mapping formula is treated as unsigned (part of the compiler's ABI), so the
+ * end of the shadow memory region is at a large positive offset from
+ * KASAN_SHADOW_OFFSET. For Software Tag-Based KASAN, the address in the
+ * formula is treated as signed. Since all kernel addresses are negative, they
+ * map to shadow memory below KASAN_SHADOW_OFFSET, making KASAN_SHADOW_OFFSET
+ * itself the end of the shadow memory region. (User pointers are positive and
+ * would map to shadow memory above KASAN_SHADOW_OFFSET, but shadow memory is
+ * not allocated for them.)
  *
  * KASAN_SHADOW_START is defined second based on KASAN_SHADOW_END. The shadow
  * memory start must map to the lowest possible kernel virtual memory address
@@ -100,7 +108,11 @@
  */
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 #define KASAN_SHADOW_OFFSET	_AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
+#ifdef CONFIG_KASAN_GENERIC
 #define KASAN_SHADOW_END	((UL(1) << (64 - KASAN_SHADOW_SCALE_SHIFT)) + KASAN_SHADOW_OFFSET)
+#else
+#define KASAN_SHADOW_END	KASAN_SHADOW_OFFSET
+#endif
 #define _KASAN_SHADOW_START(va)	(KASAN_SHADOW_END - (UL(1) << ((va) - KASAN_SHADOW_SCALE_SHIFT)))
 #define KASAN_SHADOW_START	_KASAN_SHADOW_START(vabits_actual)
 #define PAGE_END		KASAN_SHADOW_START
diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
index b65a29440a0c..6836e571555c 100644
--- a/arch/arm64/mm/kasan_init.c
+++ b/arch/arm64/mm/kasan_init.c
@@ -198,8 +198,11 @@ static bool __init root_level_aligned(u64 addr)
 /* The early shadow maps everything to a single page of zeroes */
 asmlinkage void __init kasan_early_init(void)
 {
-	BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
-		KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC))
+		BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
+			KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
+	else
+		BUILD_BUG_ON(KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
 	BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS), SHADOW_ALIGN));
 	BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS_MIN), SHADOW_ALIGN));
 	BUILD_BUG_ON(!IS_ALIGNED(KASAN_SHADOW_END, SHADOW_ALIGN));
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 890011071f2b..b396feca714f 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -61,8 +61,14 @@ int kasan_populate_early_shadow(const void *shadow_start,
 #ifndef kasan_mem_to_shadow
 static inline void *kasan_mem_to_shadow(const void *addr)
 {
-	return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
-		+ KASAN_SHADOW_OFFSET;
+	void *scaled;
+
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC))
+		scaled = (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT);
+	else
+		scaled = (void *)((long)addr >> KASAN_SHADOW_SCALE_SHIFT);
+
+	return KASAN_SHADOW_OFFSET + scaled;
 }
 #endif
 
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 3fe77a360f1c..5766714872d3 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -645,15 +645,33 @@ void kasan_report_async(void)
  */
 void kasan_non_canonical_hook(unsigned long addr)
 {
+	unsigned long max_shadow_size = BIT(BITS_PER_LONG - KASAN_SHADOW_SCALE_SHIFT);
 	unsigned long orig_addr;
 	const char *bug_type;
 
 	/*
-	 * All addresses that came as a result of the memory-to-shadow mapping
-	 * (even for bogus pointers) must be >= KASAN_SHADOW_OFFSET.
+	 * With the default kasan_mem_to_shadow() algorithm, all addresses
+	 * returned by the memory-to-shadow mapping (even for bogus pointers)
+	 * must be within a certain displacement from KASAN_SHADOW_OFFSET.
+	 *
+	 * For Generic KASAN the displacement is unsigned so the mapping from zero
+	 * to the last kernel address needs checking.
+	 *
+	 * For Software Tag-Based KASAN, the displacement is signed, so
+	 * KASAN_SHADOW_OFFSET is the center of the range. Higher positive
+	 * addresses overflow, so the range that can't be part of
+	 * memory-to-shadow mapping is above the biggest positive address
+	 * mapping and below the lowest possible one.
 	 */
-	if (addr < KASAN_SHADOW_OFFSET)
-		return;
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
+		if (addr < KASAN_SHADOW_OFFSET ||
+		    addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
+			return;
+	} else {
+		if (addr < KASAN_SHADOW_OFFSET - max_shadow_size / 2 &&
+		    addr >= KASAN_SHADOW_OFFSET + max_shadow_size / 2)
+			return;
+	}
 
 	orig_addr = (unsigned long)kasan_shadow_to_mem((void *)addr);
 
diff --git a/scripts/gdb/linux/mm.py b/scripts/gdb/linux/mm.py
index 7571aebbe650..2e63f3dedd53 100644
--- a/scripts/gdb/linux/mm.py
+++ b/scripts/gdb/linux/mm.py
@@ -110,12 +110,13 @@ class aarch64_page_ops():
         self.KERNEL_END = gdb.parse_and_eval("_end")
 
         if constants.LX_CONFIG_KASAN_GENERIC or constants.LX_CONFIG_KASAN_SW_TAGS:
+            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
             if constants.LX_CONFIG_KASAN_GENERIC:
                 self.KASAN_SHADOW_SCALE_SHIFT = 3
+                self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
             else:
                 self.KASAN_SHADOW_SCALE_SHIFT = 4
-            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
-            self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
+                self.KASAN_SHADOW_END = self.KASAN_SHADOW_OFFSET
             self.PAGE_END = self.KASAN_SHADOW_END - (1 << (self.vabits_actual - self.KASAN_SHADOW_SCALE_SHIFT))
         else:
             self.PAGE_END = self._PAGE_END(self.VA_BITS_MIN)
-- 
2.47.1

[PATCH v2 02/14] kasan: sw_tags: Check kasan_flag_enabled at runtime

[Maciej Wieczor-Retman]

From: Samuel Holland <samuel.holland@sifive.com>

On RISC-V, the ISA extension required to dereference tagged pointers is
optional, and the interface to enable pointer masking requires firmware
support. Therefore, we must detect at runtime if sw_tags is usable on a
given machine. Reuse the logic from hw_tags to dynamically enable KASAN.

This commit makes no functional change to the KASAN_HW_TAGS code path.

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 include/linux/kasan-enabled.h | 15 +++++----------
 mm/kasan/hw_tags.c            | 10 ----------
 mm/kasan/tags.c               | 10 ++++++++++
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/linux/kasan-enabled.h b/include/linux/kasan-enabled.h
index 6f612d69ea0c..648bda9495b7 100644
--- a/include/linux/kasan-enabled.h
+++ b/include/linux/kasan-enabled.h
@@ -4,7 +4,7 @@
 
 #include <linux/static_key.h>
 
-#ifdef CONFIG_KASAN_HW_TAGS
+#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
 
 DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
 
@@ -13,23 +13,18 @@ static __always_inline bool kasan_enabled(void)
 	return static_branch_likely(&kasan_flag_enabled);
 }
 
-static inline bool kasan_hw_tags_enabled(void)
-{
-	return kasan_enabled();
-}
-
-#else /* CONFIG_KASAN_HW_TAGS */
+#else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
 
 static inline bool kasan_enabled(void)
 {
 	return IS_ENABLED(CONFIG_KASAN);
 }
 
+#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
+
 static inline bool kasan_hw_tags_enabled(void)
 {
-	return false;
+	return IS_ENABLED(CONFIG_KASAN_HW_TAGS) && kasan_enabled();
 }
 
-#endif /* CONFIG_KASAN_HW_TAGS */
-
 #endif /* LINUX_KASAN_ENABLED_H */
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 9a6927394b54..7f82af13b6a6 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -45,13 +45,6 @@ static enum kasan_arg kasan_arg __ro_after_init;
 static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
 static enum kasan_arg_vmalloc kasan_arg_vmalloc __initdata;
 
-/*
- * Whether KASAN is enabled at all.
- * The value remains false until KASAN is initialized by kasan_init_hw_tags().
- */
-DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
-EXPORT_SYMBOL(kasan_flag_enabled);
-
 /*
  * Whether the selected mode is synchronous, asynchronous, or asymmetric.
  * Defaults to KASAN_MODE_SYNC.
@@ -259,9 +252,6 @@ void __init kasan_init_hw_tags(void)
 
 	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",
 		kasan_mode_info(),
 		str_on_off(kasan_vmalloc_enabled()),
diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
index d65d48b85f90..c111d98961ed 100644
--- a/mm/kasan/tags.c
+++ b/mm/kasan/tags.c
@@ -32,6 +32,13 @@ enum kasan_arg_stacktrace {
 
 static enum kasan_arg_stacktrace kasan_arg_stacktrace __initdata;
 
+/*
+ * Whether KASAN is enabled at all.
+ * The value remains false until KASAN is initialized by kasan_init_tags().
+ */
+DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
+EXPORT_SYMBOL(kasan_flag_enabled);
+
 /* Whether to collect alloc/free stack traces. */
 DEFINE_STATIC_KEY_TRUE(kasan_flag_stacktrace);
 
@@ -92,6 +99,9 @@ void __init kasan_init_tags(void)
 		if (WARN_ON(!stack_ring.entries))
 			static_branch_disable(&kasan_flag_stacktrace);
 	}
+
+	/* KASAN is now initialized, enable it. */
+	static_branch_enable(&kasan_flag_enabled);
 }
 
 static void save_stack_info(struct kmem_cache *cache, void *object,
-- 
2.47.1

[PATCH v2 03/14] kasan: sw_tags: Support outline stack tag generation

[Maciej Wieczor-Retman]

From: Samuel Holland <samuel.holland@sifive.com>

This allows stack tagging to be disabled at runtime by tagging all
stack objects with the match-all tag. This is necessary on RISC-V,
where a kernel with KASAN_SW_TAGS enabled is expected to boot on
hardware without pointer masking support.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 mm/kasan/kasan.h   | 2 ++
 mm/kasan/sw_tags.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 129178be5e64..2fb26f74dff9 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -636,6 +636,8 @@ void *__asan_memset(void *addr, int c, ssize_t len);
 void *__asan_memmove(void *dest, const void *src, ssize_t len);
 void *__asan_memcpy(void *dest, const void *src, ssize_t len);
 
+u8 __hwasan_generate_tag(void);
+
 void __hwasan_load1_noabort(void *);
 void __hwasan_store1_noabort(void *);
 void __hwasan_load2_noabort(void *);
diff --git a/mm/kasan/sw_tags.c b/mm/kasan/sw_tags.c
index b9382b5b6a37..94465a8a3640 100644
--- a/mm/kasan/sw_tags.c
+++ b/mm/kasan/sw_tags.c
@@ -71,6 +71,15 @@ u8 kasan_random_tag(void)
 	return (u8)(state % (KASAN_TAG_MAX + 1));
 }
 
+u8 __hwasan_generate_tag(void)
+{
+	if (!kasan_enabled())
+		return KASAN_TAG_KERNEL;
+
+	return kasan_random_tag();
+}
+EXPORT_SYMBOL(__hwasan_generate_tag);
+
 bool kasan_check_range(const void *addr, size_t size, bool write,
 			unsigned long ret_ip)
 {
-- 
2.47.1

[PATCH v2 04/14] kasan: sw_tags: Support tag widths less than 8 bits

[Maciej Wieczor-Retman]

From: Samuel Holland <samuel.holland@sifive.com>

Allow architectures to override KASAN_TAG_KERNEL in asm/kasan.h. This
is needed on RISC-V, which supports 57-bit virtual addresses and 7-bit
pointer tags. For consistency, move the arm64 MTE definition of
KASAN_TAG_MIN to asm/kasan.h, since it is also architecture-dependent;
RISC-V's equivalent extension is expected to support 7-bit hardware
memory tags.

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 arch/arm64/include/asm/kasan.h   |  6 ++++--
 arch/arm64/include/asm/uaccess.h |  1 +
 include/linux/kasan-tags.h       | 13 ++++++++-----
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kasan.h b/arch/arm64/include/asm/kasan.h
index e1b57c13f8a4..4ab419df8b93 100644
--- a/arch/arm64/include/asm/kasan.h
+++ b/arch/arm64/include/asm/kasan.h
@@ -6,8 +6,10 @@
 
 #include <linux/linkage.h>
 #include <asm/memory.h>
-#include <asm/mte-kasan.h>
-#include <asm/pgtable-types.h>
+
+#ifdef CONFIG_KASAN_HW_TAGS
+#define KASAN_TAG_MIN			0xF0 /* minimum value for random tags */
+#endif
 
 #define arch_kasan_set_tag(addr, tag)	__tag_set(addr, tag)
 #define arch_kasan_reset_tag(addr)	__tag_reset(addr)
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 5b91803201ef..f890dadc7b4e 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -22,6 +22,7 @@
 #include <asm/cpufeature.h>
 #include <asm/mmu.h>
 #include <asm/mte.h>
+#include <asm/mte-kasan.h>
 #include <asm/ptrace.h>
 #include <asm/memory.h>
 #include <asm/extable.h>
diff --git a/include/linux/kasan-tags.h b/include/linux/kasan-tags.h
index 4f85f562512c..e07c896f95d3 100644
--- a/include/linux/kasan-tags.h
+++ b/include/linux/kasan-tags.h
@@ -2,13 +2,16 @@
 #ifndef _LINUX_KASAN_TAGS_H
 #define _LINUX_KASAN_TAGS_H
 
+#include <asm/kasan.h>
+
+#ifndef KASAN_TAG_KERNEL
 #define KASAN_TAG_KERNEL	0xFF /* native kernel pointers tag */
-#define KASAN_TAG_INVALID	0xFE /* inaccessible memory tag */
-#define KASAN_TAG_MAX		0xFD /* maximum value for random tags */
+#endif
+
+#define KASAN_TAG_INVALID	(KASAN_TAG_KERNEL - 1) /* inaccessible memory tag */
+#define KASAN_TAG_MAX		(KASAN_TAG_KERNEL - 2) /* maximum value for random tags */
 
-#ifdef CONFIG_KASAN_HW_TAGS
-#define KASAN_TAG_MIN		0xF0 /* minimum value for random tags */
-#else
+#ifndef KASAN_TAG_MIN
 #define KASAN_TAG_MIN		0x00 /* minimum value for random tags */
 #endif
 
-- 
2.47.1

[PATCH v2 05/14] kasan: arm64: x86: Make special tags arch specific

[Maciej Wieczor-Retman]

KASAN's tag-based mode defines multiple special tag values. They're
reserved for:
- Native kernel value. On arm64 it's 0xFF and it causes an early return
  in the tag checking function.
- Invalid value. 0xFE marks an area as freed / unallocated. It's also
  the value that is used to initialize regions of shadow memory.
- Max value. 0xFD is the highest value that can be randomly generated
  for a new tag.

Metadata macro is also defined:
- Tag width equal to 8.

Tag-based mode on x86 is going to use 4 bit wide tags so all the above
values need to be changed accordingly.

Make native kernel tag arch specific for x86 and arm64.

Replace hardcoded kernel tag value and tag width with macros in KASAN's
non-arch specific code.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Remove risc-v from the patch.

 MAINTAINERS                         | 2 +-
 arch/arm64/include/asm/kasan-tags.h | 9 +++++++++
 arch/x86/include/asm/kasan-tags.h   | 9 +++++++++
 include/linux/kasan-tags.h          | 8 +++++++-
 include/linux/kasan.h               | 4 +++-
 include/linux/mm.h                  | 6 +++---
 include/linux/page-flags-layout.h   | 7 +------
 7 files changed, 33 insertions(+), 12 deletions(-)
 create mode 100644 arch/arm64/include/asm/kasan-tags.h
 create mode 100644 arch/x86/include/asm/kasan-tags.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..37971952c24b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12478,7 +12478,7 @@ L:	kasan-dev@googlegroups.com
 S:	Maintained
 B:	https://bugzilla.kernel.org/buglist.cgi?component=Sanitizers&product=Memory%20Management
 F:	Documentation/dev-tools/kasan.rst
-F:	arch/*/include/asm/*kasan.h
+F:	arch/*/include/asm/*kasan*.h
 F:	arch/*/mm/kasan_init*
 F:	include/linux/kasan*.h
 F:	lib/Kconfig.kasan
diff --git a/arch/arm64/include/asm/kasan-tags.h b/arch/arm64/include/asm/kasan-tags.h
new file mode 100644
index 000000000000..9e835da95f6b
--- /dev/null
+++ b/arch/arm64/include/asm/kasan-tags.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_KASAN_TAGS_H
+#define __ASM_KASAN_TAGS_H
+
+#define KASAN_TAG_KERNEL	0xFF /* native kernel pointers tag */
+
+#define KASAN_TAG_WIDTH 8
+
+#endif /* ASM_KASAN_TAGS_H */
diff --git a/arch/x86/include/asm/kasan-tags.h b/arch/x86/include/asm/kasan-tags.h
new file mode 100644
index 000000000000..68ba385bc75c
--- /dev/null
+++ b/arch/x86/include/asm/kasan-tags.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_KASAN_TAGS_H
+#define __ASM_KASAN_TAGS_H
+
+#define KASAN_TAG_KERNEL	0xF /* native kernel pointers tag */
+
+#define KASAN_TAG_WIDTH		4
+
+#endif /* ASM_KASAN_TAGS_H */
diff --git a/include/linux/kasan-tags.h b/include/linux/kasan-tags.h
index e07c896f95d3..ad5c11950233 100644
--- a/include/linux/kasan-tags.h
+++ b/include/linux/kasan-tags.h
@@ -2,7 +2,13 @@
 #ifndef _LINUX_KASAN_TAGS_H
 #define _LINUX_KASAN_TAGS_H
 
-#include <asm/kasan.h>
+#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
+#include <asm/kasan-tags.h>
+#endif
+
+#ifndef KASAN_TAG_WIDTH
+#define KASAN_TAG_WIDTH		0
+#endif
 
 #ifndef KASAN_TAG_KERNEL
 #define KASAN_TAG_KERNEL	0xFF /* native kernel pointers tag */
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index b396feca714f..54481f8c30c5 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -40,7 +40,9 @@ typedef unsigned int __bitwise kasan_vmalloc_flags_t;
 
 #ifdef CONFIG_KASAN_SW_TAGS
 /* This matches KASAN_TAG_INVALID. */
-#define KASAN_SHADOW_INIT 0xFE
+#ifndef KASAN_SHADOW_INIT
+#define KASAN_SHADOW_INIT KASAN_TAG_INVALID
+#endif
 #else
 #define KASAN_SHADOW_INIT 0
 #endif
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7b1068ddcbb7..0b1d21864294 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1824,7 +1824,7 @@ static inline u8 page_kasan_tag(const struct page *page)
 
 	if (kasan_enabled()) {
 		tag = (page->flags >> KASAN_TAG_PGSHIFT) & KASAN_TAG_MASK;
-		tag ^= 0xff;
+		tag ^= KASAN_TAG_KERNEL;
 	}
 
 	return tag;
@@ -1837,7 +1837,7 @@ static inline void page_kasan_tag_set(struct page *page, u8 tag)
 	if (!kasan_enabled())
 		return;
 
-	tag ^= 0xff;
+	tag ^= KASAN_TAG_KERNEL;
 	old_flags = READ_ONCE(page->flags);
 	do {
 		flags = old_flags;
@@ -1856,7 +1856,7 @@ static inline void page_kasan_tag_reset(struct page *page)
 
 static inline u8 page_kasan_tag(const struct page *page)
 {
-	return 0xff;
+	return KASAN_TAG_KERNEL;
 }
 
 static inline void page_kasan_tag_set(struct page *page, u8 tag) { }
diff --git a/include/linux/page-flags-layout.h b/include/linux/page-flags-layout.h
index 4f5c9e979bb9..b2cc4cb870e0 100644
--- a/include/linux/page-flags-layout.h
+++ b/include/linux/page-flags-layout.h
@@ -3,6 +3,7 @@
 #define PAGE_FLAGS_LAYOUT_H
 
 #include <linux/numa.h>
+#include <linux/kasan-tags.h>
 #include <generated/bounds.h>
 
 /*
@@ -72,12 +73,6 @@
 #define NODE_NOT_IN_PAGE_FLAGS	1
 #endif
 
-#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
-#define KASAN_TAG_WIDTH 8
-#else
-#define KASAN_TAG_WIDTH 0
-#endif
-
 #ifdef CONFIG_NUMA_BALANCING
 #define LAST__PID_SHIFT 8
 #define LAST__PID_MASK  ((1 << LAST__PID_SHIFT)-1)
-- 
2.47.1

[PATCH v2 06/14] x86: Add arch specific kasan functions

[Maciej Wieczor-Retman]

KASAN's software tag-based mode needs multiple macros/functions to
handle tag and pointer interactions - mainly to set and retrieve tags
from the top bits of a pointer.

Mimic functions currently used by arm64 but change the tag's position to
bits [60:57] in the pointer.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 arch/x86/include/asm/kasan.h | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
index de75306b932e..8829337a75fa 100644
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -3,6 +3,8 @@
 #define _ASM_X86_KASAN_H
 
 #include <linux/const.h>
+#include <linux/kasan-tags.h>
+#include <linux/types.h>
 #define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
 #define KASAN_SHADOW_SCALE_SHIFT 3
 
@@ -24,8 +26,33 @@
 						  KASAN_SHADOW_SCALE_SHIFT)))
 
 #ifndef __ASSEMBLY__
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+
+#define arch_kasan_set_tag(addr, tag)	__tag_set(addr, tag)
+#define arch_kasan_reset_tag(addr)	__tag_reset(addr)
+#define arch_kasan_get_tag(addr)	__tag_get(addr)
+
+#ifdef CONFIG_KASAN_SW_TAGS
+
+#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))
+#else
+#define __tag_shifted(tag)		0UL
+#define __tag_reset(addr)		(addr)
+#define __tag_get(addr)			0
+#endif /* CONFIG_KASAN_SW_TAGS */
 
 #ifdef CONFIG_KASAN
+
+static inline const void *__tag_set(const void *addr, u8 tag)
+{
+	u64 __addr = (u64)addr & ~__tag_shifted(KASAN_TAG_KERNEL);
+	return (const void *)(__addr | __tag_shifted(tag));
+}
+
 void __init kasan_early_init(void);
 void __init kasan_init(void);
 void __init kasan_populate_shadow_for_vaddr(void *va, size_t size, int nid);
@@ -34,8 +61,9 @@ static inline void kasan_early_init(void) { }
 static inline void kasan_init(void) { }
 static inline void kasan_populate_shadow_for_vaddr(void *va, size_t size,
 						   int nid) { }
-#endif
 
-#endif
+#endif /* CONFIG_KASAN */
+
+#endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.47.1

[PATCH v2 07/14] x86: Reset tag for virtual to physical address conversions

[Maciej Wieczor-Retman]

Any place where pointer arithmetic is used to convert a virtual address
into a physical one can raise errors if the virtual address is tagged.

Reset the pointer's tag by sign extending the tag bits in macros that do
pointer arithmetic in address conversions. There will be no change in
compiled code with KASAN disabled since the compiler will optimize the
__tag_reset() out.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 arch/x86/include/asm/page.h    | 17 +++++++++++++----
 arch/x86/include/asm/page_64.h |  2 +-
 arch/x86/mm/physaddr.c         |  1 +
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index c9fe207916f4..fdafeb06c195 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -7,6 +7,7 @@
 #ifdef __KERNEL__
 
 #include <asm/page_types.h>
+#include <asm/kasan.h>
 
 #ifdef CONFIG_X86_64
 #include <asm/page_64.h>
@@ -41,7 +42,7 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
 #define __pa(x)		__phys_addr((unsigned long)(x))
 #endif
 
-#define __pa_nodebug(x)	__phys_addr_nodebug((unsigned long)(x))
+#define __pa_nodebug(x)	__phys_addr_nodebug((unsigned long)(__tag_reset(x)))
 /* __pa_symbol should be used for C visible symbols.
    This seems to be the official gcc blessed way to do such arithmetic. */
 /*
@@ -65,9 +66,17 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
  * virt_to_page(kaddr) returns a valid pointer if and only if
  * virt_addr_valid(kaddr) returns true.
  */
-#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
+
+#ifdef CONFIG_KASAN_SW_TAGS
+#define page_to_virt(x)	({									\
+	__typeof__(x) __page = x;								\
+	void *__addr = __va(page_to_pfn((__typeof__(x))__tag_reset(__page)) << PAGE_SHIFT);	\
+	(void *)__tag_set((const void *)__addr, page_kasan_tag(__page));			\
+})
+#endif
+#define virt_to_page(kaddr)	pfn_to_page(__pa((void *)__tag_reset(kaddr)) >> PAGE_SHIFT)
 extern bool __virt_addr_valid(unsigned long kaddr);
-#define virt_addr_valid(kaddr)	__virt_addr_valid((unsigned long) (kaddr))
+#define virt_addr_valid(kaddr)	__virt_addr_valid((unsigned long)(__tag_reset(kaddr)))
 
 static __always_inline void *pfn_to_kaddr(unsigned long pfn)
 {
@@ -81,7 +90,7 @@ static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
 
 static __always_inline u64 __is_canonical_address(u64 vaddr, u8 vaddr_bits)
 {
-	return __canonical_address(vaddr, vaddr_bits) == vaddr;
+	return __canonical_address(vaddr, vaddr_bits) == __tag_reset(vaddr);
 }
 
 #endif	/* __ASSEMBLY__ */
diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h
index d63576608ce7..54990ea82f74 100644
--- a/arch/x86/include/asm/page_64.h
+++ b/arch/x86/include/asm/page_64.h
@@ -33,7 +33,7 @@ static __always_inline unsigned long __phys_addr_nodebug(unsigned long x)
 extern unsigned long __phys_addr(unsigned long);
 extern unsigned long __phys_addr_symbol(unsigned long);
 #else
-#define __phys_addr(x)		__phys_addr_nodebug(x)
+#define __phys_addr(x)		__phys_addr_nodebug(__tag_reset(x))
 #define __phys_addr_symbol(x) \
 	((unsigned long)(x) - __START_KERNEL_map + phys_base)
 #endif
diff --git a/arch/x86/mm/physaddr.c b/arch/x86/mm/physaddr.c
index fc3f3d3e2ef2..7f2b11308245 100644
--- a/arch/x86/mm/physaddr.c
+++ b/arch/x86/mm/physaddr.c
@@ -14,6 +14,7 @@
 #ifdef CONFIG_DEBUG_VIRTUAL
 unsigned long __phys_addr(unsigned long x)
 {
+	x = __tag_reset(x);
 	unsigned long y = x - __START_KERNEL_map;
 
 	/* use the carry flag to determine if x was < __START_KERNEL_map */
-- 
2.47.1

[PATCH v2 08/14] x86: Physical address comparisons in fill_p*d/pte

[Maciej Wieczor-Retman]

Calculating page offset returns a pointer without a tag. When comparing
the calculated offset to a tagged page pointer an error is raised
because they are not equal.

Change pointer comparisons to physical address comparisons as to avoid
issues with tagged pointers that pointer arithmetic would create. Open
code pte_offset_kernel(), pmd_offset(), pud_offset() and p4d_offset().
Because one parameter is always zero and the rest of the function
insides are enclosed inside __va(), removing that layer lowers the
complexity of final assembly.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Open code *_offset() to avoid it's internal __va().

 arch/x86/mm/init_64.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 01ea7c6df303..e555895dbb68 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -251,7 +251,10 @@ static p4d_t *fill_p4d(pgd_t *pgd, unsigned long vaddr)
 	if (pgd_none(*pgd)) {
 		p4d_t *p4d = (p4d_t *)spp_getpage();
 		pgd_populate(&init_mm, pgd, p4d);
-		if (p4d != p4d_offset(pgd, 0))
+
+		if (__pa(p4d) != (pgtable_l5_enabled() ?
+				  __pa(pgd) :
+				  (unsigned long)pgd_val(*pgd) & PTE_PFN_MASK))
 			printk(KERN_ERR "PAGETABLE BUG #00! %p <-> %p\n",
 			       p4d, p4d_offset(pgd, 0));
 	}
@@ -263,7 +266,7 @@ static pud_t *fill_pud(p4d_t *p4d, unsigned long vaddr)
 	if (p4d_none(*p4d)) {
 		pud_t *pud = (pud_t *)spp_getpage();
 		p4d_populate(&init_mm, p4d, pud);
-		if (pud != pud_offset(p4d, 0))
+		if (__pa(pud) != (p4d_val(*p4d) & p4d_pfn_mask(*p4d)))
 			printk(KERN_ERR "PAGETABLE BUG #01! %p <-> %p\n",
 			       pud, pud_offset(p4d, 0));
 	}
@@ -275,7 +278,7 @@ static pmd_t *fill_pmd(pud_t *pud, unsigned long vaddr)
 	if (pud_none(*pud)) {
 		pmd_t *pmd = (pmd_t *) spp_getpage();
 		pud_populate(&init_mm, pud, pmd);
-		if (pmd != pmd_offset(pud, 0))
+		if (__pa(pmd) != (pud_val(*pud) & pud_pfn_mask(*pud)))
 			printk(KERN_ERR "PAGETABLE BUG #02! %p <-> %p\n",
 			       pmd, pmd_offset(pud, 0));
 	}
@@ -287,7 +290,7 @@ static pte_t *fill_pte(pmd_t *pmd, unsigned long vaddr)
 	if (pmd_none(*pmd)) {
 		pte_t *pte = (pte_t *) spp_getpage();
 		pmd_populate_kernel(&init_mm, pmd, pte);
-		if (pte != pte_offset_kernel(pmd, 0))
+		if (__pa(pte) != (pmd_val(*pmd) & pmd_pfn_mask(*pmd)))
 			printk(KERN_ERR "PAGETABLE BUG #03!\n");
 	}
 	return pte_offset_kernel(pmd, vaddr);
-- 
2.47.1

[PATCH v2 09/14] mm: Pcpu chunk address tag reset

[Maciej Wieczor-Retman]

The problem presented here is related to NUMA systems and tag-based
KASAN mode. Getting to it can be explained in the following points:

	1. A new chunk is created with pcpu_create_chunk() and
	   vm_structs are allocated. On systems with one NUMA node only
	   one is allocated, but with more NUMA nodes at least a second
	   one will be allocated too.

	2. chunk->base_addr is assigned the modified value of
	   vms[0]->addr and thus inherits the tag of this allocated
	   structure.

	3. In pcpu_alloc() for each possible cpu pcpu_chunk_addr() is
	   executed which calculates per cpu pointers that correspond to
	   the vms structure addresses. The calculations are based on
	   adding an offset from a table to chunk->base_addr.

Here the problem presents itself since for addresses based on vms[1] and
up, the tag will be different than the ones based on vms[0] (base_addr).
The tag mismatch happens and an error is reported.

Reset the base_addr tag, since it will disable tag checks for pointers
derived arithmetically from base_addr that would inherit its tag.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 mm/percpu-vm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
index cd69caf6aa8d..e13750d804f7 100644
--- a/mm/percpu-vm.c
+++ b/mm/percpu-vm.c
@@ -347,7 +347,7 @@ static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
 	}
 
 	chunk->data = vms;
-	chunk->base_addr = vms[0]->addr - pcpu_group_offsets[0];
+	chunk->base_addr = kasan_reset_tag(vms[0]->addr) - pcpu_group_offsets[0];
 
 	pcpu_stats_chunk_alloc();
 	trace_percpu_create_chunk(chunk->base_addr);
-- 
2.47.1

[PATCH v2 10/14] x86: KASAN raw shadow memory PTE init

[Maciej Wieczor-Retman]

In KASAN's generic mode the default value in shadow memory is zero.
During initialization of shadow memory pages they are allocated and
zeroed.

In KASAN's tag-based mode the default tag for the arm64 architecture is
0xFE which corresponds to any memory that should not be accessed. On x86
(where tags are 4-bit wide instead of 8-bit wide) that tag is 0xE so
during the initializations all the bytes in shadow memory pages should
be filled with 0xE or 0xEE if two tags should be packed in one shadow
byte.

Use memblock_alloc_try_nid_raw() instead of memblock_alloc_try_nid() to
avoid zeroing out the memory so it can be set with the KASAN invalid
tag.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Remove dense mode references, use memset() instead of kasan_poison().

 arch/x86/mm/kasan_init_64.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
index 9dddf19a5571..299a2144dac4 100644
--- a/arch/x86/mm/kasan_init_64.c
+++ b/arch/x86/mm/kasan_init_64.c
@@ -35,6 +35,18 @@ static __init void *early_alloc(size_t size, int nid, bool should_panic)
 	return ptr;
 }
 
+static __init void *early_raw_alloc(size_t size, int nid, bool should_panic)
+{
+	void *ptr = memblock_alloc_try_nid_raw(size, size,
+			__pa(MAX_DMA_ADDRESS), MEMBLOCK_ALLOC_ACCESSIBLE, nid);
+
+	if (!ptr && should_panic)
+		panic("%pS: Failed to allocate page, nid=%d from=%lx\n",
+		      (void *)_RET_IP_, nid, __pa(MAX_DMA_ADDRESS));
+
+	return ptr;
+}
+
 static void __init kasan_populate_pmd(pmd_t *pmd, unsigned long addr,
 				      unsigned long end, int nid)
 {
@@ -64,8 +76,9 @@ static void __init kasan_populate_pmd(pmd_t *pmd, unsigned long addr,
 		if (!pte_none(*pte))
 			continue;
 
-		p = early_alloc(PAGE_SIZE, nid, true);
-		entry = pfn_pte(PFN_DOWN(__pa(p)), PAGE_KERNEL);
+		p = early_raw_alloc(PAGE_SIZE, nid, true);
+		memset(p, PAGE_SIZE, KASAN_SHADOW_INIT);
+		entry = pfn_pte(PFN_DOWN(__pa_nodebug(p)), PAGE_KERNEL);
 		set_pte_at(&init_mm, addr, pte, entry);
 	} while (pte++, addr += PAGE_SIZE, addr != end);
 }
@@ -437,7 +450,7 @@ void __init kasan_init(void)
 	 * it may contain some garbage. Now we can clear and write protect it,
 	 * since after the TLB flush no one should write to it.
 	 */
-	memset(kasan_early_shadow_page, 0, PAGE_SIZE);
+	memset(kasan_early_shadow_page, KASAN_SHADOW_INIT, PAGE_SIZE);
 	for (i = 0; i < PTRS_PER_PTE; i++) {
 		pte_t pte;
 		pgprot_t prot;
-- 
2.47.1

[PATCH v2 11/14] x86: LAM initialization

[Maciej Wieczor-Retman]

To make use of KASAN's tag based mode on x86 Linear Address Masking
(LAM) needs to be enabled. To do that the 28th bit in CR4 needs to be
set.

Set the bit in early memory initialization.

When launching secondary CPUs the LAM bit gets lost. To avoid this it
needs to get added in a mask in head_64.S. The bit mask permits some
bits of CR4 to pass from the primary CPU to the secondary CPUs without
being cleared.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 arch/x86/kernel/head_64.S | 3 +++
 arch/x86/mm/init.c        | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 31345e0ba006..87158729f138 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -206,6 +206,9 @@ SYM_INNER_LABEL(common_startup_64, SYM_L_LOCAL)
 	 *  there will be no global TLB entries after the execution."
 	 */
 	movl	$(X86_CR4_PAE | X86_CR4_LA57), %edx
+#ifdef CONFIG_ADDRESS_MASKING
+	orl	$X86_CR4_LAM_SUP, %edx
+#endif
 #ifdef CONFIG_X86_MCE
 	/*
 	 * Preserve CR4.MCE if the kernel will enable #MC support.
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 62aa4d66a032..5499ba683b53 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -761,6 +761,9 @@ void __init init_mem_mapping(void)
 	probe_page_size_mask();
 	setup_pcid();
 
+	if (boot_cpu_has(X86_FEATURE_LAM) && IS_ENABLED(CONFIG_KASAN_SW_TAGS))
+		cr4_set_bits_and_update_boot(X86_CR4_LAM_SUP);
+
 #ifdef CONFIG_X86_64
 	end = max_pfn << PAGE_SHIFT;
 #else
-- 
2.47.1

[PATCH v2 12/14] x86: Minimal SLAB alignment

[Maciej Wieczor-Retman]

Adjust x86 minimal SLAB alignment to match KASAN granularity size. In
tag-based mode the size changes to 16 bytes so the value needs to be 4.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 arch/x86/include/asm/kasan.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
index 8829337a75fa..a75f0748a4b6 100644
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -36,6 +36,8 @@
 
 #ifdef CONFIG_KASAN_SW_TAGS
 
+#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT)
+
 #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))
-- 
2.47.1

[PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On x86, generic KASAN is setup in a way that needs a single
KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
to facilitate boot time switching and it's a compiler ABI so it can't be
changed during runtime.

Software tag-based mode doesn't tie shadow start and end to any linear
addresses as part of the compiler ABI so it can be changed during
runtime. This notion, for KASAN purposes, allows to optimize out macros
such us pgtable_l5_enabled() which would otherwise be used in every
single KASAN related function.

Use runtime_const infrastructure with pgtable_l5_enabled() to initialize
the end address of KASAN's shadow address space. It's a good choice
since in software tag based mode KASAN_SHADOW_OFFSET and
KASAN_SHADOW_END refer to the same value and the offset in
kasan_mem_to_shadow() is a signed negative value.

Setup KASAN_SHADOW_END values so that they're aligned to 4TB in 4-level
paging mode and to 2PB in 5-level paging mode. Also update x86 memory
map documentation.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Change documentation kasan start address to non-dense values.

 Documentation/arch/x86/x86_64/mm.rst |  6 ++++--
 arch/x86/Kconfig                     |  3 +--
 arch/x86/include/asm/kasan.h         | 14 +++++++++++++-
 arch/x86/kernel/vmlinux.lds.S        |  1 +
 arch/x86/mm/kasan_init_64.c          |  5 ++++-
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/Documentation/arch/x86/x86_64/mm.rst b/Documentation/arch/x86/x86_64/mm.rst
index f2db178b353f..5014ec322e19 100644
--- a/Documentation/arch/x86/x86_64/mm.rst
+++ b/Documentation/arch/x86/x86_64/mm.rst
@@ -60,7 +60,8 @@ Complete virtual memory map with 4-level page tables
    ffffe90000000000 |  -23    TB | ffffe9ffffffffff |    1 TB | ... unused hole
    ffffea0000000000 |  -22    TB | ffffeaffffffffff |    1 TB | virtual memory map (vmemmap_base)
    ffffeb0000000000 |  -21    TB | ffffebffffffffff |    1 TB | ... unused hole
-   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory
+   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory (generic mode)
+   fffff40000000000 |   -8    TB | fffffc0000000000 |    8 TB | KASAN shadow memory (software tag-based mode)
   __________________|____________|__________________|_________|____________________________________________________________
                                                               |
                                                               | Identical layout to the 56-bit one from here on:
@@ -130,7 +131,8 @@ Complete virtual memory map with 5-level page tables
    ffd2000000000000 |  -11.5  PB | ffd3ffffffffffff |  0.5 PB | ... unused hole
    ffd4000000000000 |  -11    PB | ffd5ffffffffffff |  0.5 PB | virtual memory map (vmemmap_base)
    ffd6000000000000 |  -10.5  PB | ffdeffffffffffff | 2.25 PB | ... unused hole
-   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory
+   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory (generic mode)
+   ffe0000000000000 |   -6    PB | fff0000000000000 |    4 PB | KASAN shadow memory (software tag-based mode)
   __________________|____________|__________________|_________|____________________________________________________________
                                                               |
                                                               | Identical layout to the 47-bit one from here on:
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6df7779ed6da..f4ef64bf824a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -400,8 +400,7 @@ config AUDIT_ARCH
 
 config KASAN_SHADOW_OFFSET
 	hex
-	depends on KASAN
-	default 0xdffffc0000000000
+	default 0xdffffc0000000000 if KASAN_GENERIC
 
 config HAVE_INTEL_TXT
 	def_bool y
diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
index a75f0748a4b6..4bfd3641af84 100644
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -5,7 +5,7 @@
 #include <linux/const.h>
 #include <linux/kasan-tags.h>
 #include <linux/types.h>
-#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
+
 #define KASAN_SHADOW_SCALE_SHIFT 3
 
 /*
@@ -14,6 +14,8 @@
  * for kernel really starts from compiler's shadow offset +
  * 'kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT
  */
+#ifdef CONFIG_KASAN_GENERIC
+#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
 #define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
 					((-1UL << __VIRTUAL_MASK_SHIFT) >> \
 						KASAN_SHADOW_SCALE_SHIFT))
@@ -24,12 +26,22 @@
 #define KASAN_SHADOW_END        (KASAN_SHADOW_START + \
 					(1ULL << (__VIRTUAL_MASK_SHIFT - \
 						  KASAN_SHADOW_SCALE_SHIFT)))
+#endif
+
 
 #ifndef __ASSEMBLY__
+#include <asm/runtime-const.h>
 #include <linux/bitops.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 
+#ifdef CONFIG_KASAN_SW_TAGS
+extern unsigned long KASAN_SHADOW_END_RC;
+#define KASAN_SHADOW_END	runtime_const_ptr(KASAN_SHADOW_END_RC)
+#define KASAN_SHADOW_OFFSET	KASAN_SHADOW_END
+#define KASAN_SHADOW_START	(KASAN_SHADOW_END - ((UL(1)) << (__VIRTUAL_MASK_SHIFT - KASAN_SHADOW_SCALE_SHIFT)))
+#endif
+
 #define arch_kasan_set_tag(addr, tag)	__tag_set(addr, tag)
 #define arch_kasan_reset_tag(addr)	__tag_reset(addr)
 #define arch_kasan_get_tag(addr)	__tag_get(addr)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 0deb4887d6e9..df6c85f8f48f 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -353,6 +353,7 @@ SECTIONS
 
 	RUNTIME_CONST_VARIABLES
 	RUNTIME_CONST(ptr, USER_PTR_MAX)
+	RUNTIME_CONST(ptr, KASAN_SHADOW_END_RC)
 
 	. = ALIGN(PAGE_SIZE);
 
diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
index 299a2144dac4..5ca5862a5cd6 100644
--- a/arch/x86/mm/kasan_init_64.c
+++ b/arch/x86/mm/kasan_init_64.c
@@ -358,6 +358,9 @@ void __init kasan_init(void)
 	int i;
 
 	memcpy(early_top_pgt, init_top_pgt, sizeof(early_top_pgt));
+	unsigned long KASAN_SHADOW_END_RC = pgtable_l5_enabled() ? 0xfff0000000000000 : 0xfffffc0000000000;
+
+	runtime_const_init(ptr, KASAN_SHADOW_END_RC);
 
 	/*
 	 * We use the same shadow offset for 4- and 5-level paging to
@@ -372,7 +375,7 @@ void __init kasan_init(void)
 	 * bunch of things like kernel code, modules, EFI mapping, etc.
 	 * We need to take extra steps to not overwrite them.
 	 */
-	if (pgtable_l5_enabled()) {
+	if (pgtable_l5_enabled() && !IS_ENABLED(CONFIG_KASAN_SW_TAGS)) {
 		void *ptr;
 
 		ptr = (void *)pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_END));
-- 
2.47.1

[PATCH v2 14/14] x86: Make software tag-based kasan available

[Maciej Wieczor-Retman]

Make CONFIG_KASAN_SW_TAGS available for x86 machines if they have
ADDRESS_MASKING enabled (LAM) as that works similarly to Top-Byte Ignore
(TBI) that allows the software tag-based mode on arm64 platform.

Set scale macro based on KASAN mode: in software tag-based mode 32 bytes
of memory map to one shadow byte and 16 in generic mode.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Remove KASAN dense code.

 arch/x86/Kconfig                | 6 ++++++
 arch/x86/boot/compressed/misc.h | 1 +
 arch/x86/include/asm/kasan.h    | 2 +-
 arch/x86/kernel/setup.c         | 2 ++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f4ef64bf824a..dc48eb5b664f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -195,6 +195,7 @@ config X86
 	select HAVE_ARCH_JUMP_LABEL_RELATIVE
 	select HAVE_ARCH_KASAN			if X86_64
 	select HAVE_ARCH_KASAN_VMALLOC		if X86_64
+	select HAVE_ARCH_KASAN_SW_TAGS		if ADDRESS_MASKING
 	select HAVE_ARCH_KFENCE
 	select HAVE_ARCH_KMSAN			if X86_64
 	select HAVE_ARCH_KGDB
@@ -402,6 +403,11 @@ config KASAN_SHADOW_OFFSET
 	hex
 	default 0xdffffc0000000000 if KASAN_GENERIC
 
+config KASAN_SHADOW_SCALE_SHIFT
+	int
+	default 4 if KASAN_SW_TAGS
+	default 3
+
 config HAVE_INTEL_TXT
 	def_bool y
 	depends on INTEL_IOMMU && ACPI
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index dd8d1a85f671..f6a87e9ad200 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -13,6 +13,7 @@
 #undef CONFIG_PARAVIRT_SPINLOCKS
 #undef CONFIG_KASAN
 #undef CONFIG_KASAN_GENERIC
+#undef CONFIG_KASAN_SW_TAGS
 
 #define __NO_FORTIFY
 
diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
index 4bfd3641af84..cfc31e4a2f70 100644
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -6,7 +6,7 @@
 #include <linux/kasan-tags.h>
 #include <linux/types.h>
 
-#define KASAN_SHADOW_SCALE_SHIFT 3
+#define KASAN_SHADOW_SCALE_SHIFT CONFIG_KASAN_SHADOW_SCALE_SHIFT
 
 /*
  * Compiler uses shadow offset assuming that addresses start
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index cebee310e200..768990c573ea 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1124,6 +1124,8 @@ void __init setup_arch(char **cmdline_p)
 
 	kasan_init();
 
+	kasan_init_sw_tags();
+
 	/*
 	 * Sync back kernel address range.
 	 *
-- 
2.47.1

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:16 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> From: Samuel Holland <samuel.holland@sifive.com>
>
> Currently, kasan_mem_to_shadow() uses a logical right shift, which turns
> canonical kernel addresses into non-canonical addresses by clearing the
> high KASAN_SHADOW_SCALE_SHIFT bits. The value of KASAN_SHADOW_OFFSET is
> then chosen so that the addition results in a canonical address for the
> shadow memory.
>
> For KASAN_GENERIC, this shift/add combination is ABI with the compiler,
> because KASAN_SHADOW_OFFSET is used in compiler-generated inline tag
> checks[1], which must only attempt to dereference canonical addresses.
>
> However, for KASAN_SW_TAGS we have some freedom to change the algorithm
> without breaking the ABI. Because TBI is enabled for kernel addresses,
> the top bits of shadow memory addresses computed during tag checks are
> irrelevant, and so likewise are the top bits of KASAN_SHADOW_OFFSET.
> This is demonstrated by the fact that LLVM uses a logical right shift
> in the tag check fast path[2] but a sbfx (signed bitfield extract)
> instruction in the slow path[3] without causing any issues.
>
> Using an arithmetic shift in kasan_mem_to_shadow() provides a number of
> benefits:
>
> 1) The memory layout is easier to understand. KASAN_SHADOW_OFFSET
> becomes a canonical memory address, and the shifted pointer becomes a
> negative offset, so KASAN_SHADOW_OFFSET == KASAN_SHADOW_END regardless
> of the shift amount or the size of the virtual address space.
>
> 2) KASAN_SHADOW_OFFSET becomes a simpler constant, requiring only one
> instruction to load instead of two. Since it must be loaded in each
> function with a tag check, this decreases kernel text size by 0.5%.
>
> 3) This shift and the sign extension from kasan_reset_tag() can be
> combined into a single sbfx instruction. When this same algorithm change
> is applied to the compiler, it removes an instruction from each inline
> tag check, further reducing kernel text size by an additional 4.6%.
>
> These benefits extend to other architectures as well. On RISC-V, where
> the baseline ISA does not shifted addition or have an equivalent to the
> sbfx instruction, loading KASAN_SHADOW_OFFSET is reduced from 3 to 2
> instructions, and kasan_mem_to_shadow(kasan_reset_tag(addr)) similarly
> combines two consecutive right shifts.
>
> Due to signed memory-to-shadow mapping kasan_non_canonical_hook() needs
> changes - specifically the first part that tries to deduce if a faulty
> address came from kasan_mem_to_shadow(). Previous value of
> KASAN_SHADOW_OFFSET prevented any overflows when trying to map the
> entire linear address space to shadow memory so the check in
> kasan_non_canonical_hook() could consist of only checking whether the
> address isn't below KASAN_SHADOW_OFFSET.
>
> The signed memory-to-shadow conversion means negative addresses will be
> mapped below KASAN_SHADOW_OFFSET and positive addresses will map above
> KASAN_SHADOW_OFFSET. When looking at the mapping of the entire address
> space there will be an overflow when a big enough positive address will
> be passed to kasan_mem_to_shadow(). Then the question of finding
> addresses that couldn't come from kasan_mem_to_shadow() can be reduced
> to figuring out if the address isn't above the highest overflowed value
> (most positive address possible) AND below the most negative address
> possible.

Is there any reason we need this change for x86 SW_TAGS besides the
optimization benefits?

Is it required for the "x86: runtime_const used for KASAN_SHADOW_END"
patch? If so, please check my comment there first.

>
> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L1316 [1]
> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L895 [2]
> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp#L669 [3]
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v2: (Maciej)
> - Correct address range that's checked in kasan_non_canonical_hook().
>   Adjust the comment inside.
> - Remove part of comment from arch/arm64/include/asm/memory.h.
> - Append patch message paragraph about the overflow in
>   kasan_non_canonical_hook().
>
>  arch/arm64/Kconfig              | 10 +++++-----
>  arch/arm64/include/asm/memory.h | 14 +++++++++++++-
>  arch/arm64/mm/kasan_init.c      |  7 +++++--
>  include/linux/kasan.h           | 10 ++++++++--
>  mm/kasan/report.c               | 26 ++++++++++++++++++++++----
>  scripts/gdb/linux/mm.py         |  5 +++--
>  6 files changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fcdd0ed3eca8..fe7d79b447c3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -426,11 +426,11 @@ config KASAN_SHADOW_OFFSET
>         default 0xdffffe0000000000 if ARM64_VA_BITS_42 && !KASAN_SW_TAGS
>         default 0xdfffffc000000000 if ARM64_VA_BITS_39 && !KASAN_SW_TAGS
>         default 0xdffffff800000000 if ARM64_VA_BITS_36 && !KASAN_SW_TAGS
> -       default 0xefff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
> -       default 0xefffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
> -       default 0xeffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
> -       default 0xefffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
> -       default 0xeffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
> +       default 0xffff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
> +       default 0xffffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
> +       default 0xfffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
> +       default 0xffffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
> +       default 0xfffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS

Ah, we also need to update Documentation/arch/arm64/kasan-offsets.sh,
these offsets are generated by that script.

Let's also point out in the commit message, that this change does not
move the location of the shadow memory but only changes the way that
location is calculated.

>         default 0xffffffffffffffff
>
>  config UNWIND_TABLES
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 717829df294e..e71cdf036287 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -89,7 +89,15 @@
>   *
>   * KASAN_SHADOW_END is defined first as the shadow address that corresponds to
>   * the upper bound of possible virtual kernel memory addresses UL(1) << 64
> - * according to the mapping formula.
> + * according to the mapping formula. For Generic KASAN, the address in the
> + * mapping formula is treated as unsigned (part of the compiler's ABI), so the
> + * end of the shadow memory region is at a large positive offset from
> + * KASAN_SHADOW_OFFSET. For Software Tag-Based KASAN, the address in the
> + * formula is treated as signed. Since all kernel addresses are negative, they
> + * map to shadow memory below KASAN_SHADOW_OFFSET, making KASAN_SHADOW_OFFSET
> + * itself the end of the shadow memory region. (User pointers are positive and
> + * would map to shadow memory above KASAN_SHADOW_OFFSET, but shadow memory is
> + * not allocated for them.)
>   *
>   * KASAN_SHADOW_START is defined second based on KASAN_SHADOW_END. The shadow
>   * memory start must map to the lowest possible kernel virtual memory address
> @@ -100,7 +108,11 @@
>   */
>  #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
>  #define KASAN_SHADOW_OFFSET    _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
> +#ifdef CONFIG_KASAN_GENERIC
>  #define KASAN_SHADOW_END       ((UL(1) << (64 - KASAN_SHADOW_SCALE_SHIFT)) + KASAN_SHADOW_OFFSET)
> +#else
> +#define KASAN_SHADOW_END       KASAN_SHADOW_OFFSET
> +#endif
>  #define _KASAN_SHADOW_START(va)        (KASAN_SHADOW_END - (UL(1) << ((va) - KASAN_SHADOW_SCALE_SHIFT)))
>  #define KASAN_SHADOW_START     _KASAN_SHADOW_START(vabits_actual)
>  #define PAGE_END               KASAN_SHADOW_START
> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> index b65a29440a0c..6836e571555c 100644
> --- a/arch/arm64/mm/kasan_init.c
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -198,8 +198,11 @@ static bool __init root_level_aligned(u64 addr)
>  /* The early shadow maps everything to a single page of zeroes */
>  asmlinkage void __init kasan_early_init(void)
>  {
> -       BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
> -               KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
> +               BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
> +                       KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
> +       else
> +               BUILD_BUG_ON(KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
>         BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS), SHADOW_ALIGN));
>         BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS_MIN), SHADOW_ALIGN));
>         BUILD_BUG_ON(!IS_ALIGNED(KASAN_SHADOW_END, SHADOW_ALIGN));
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index 890011071f2b..b396feca714f 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -61,8 +61,14 @@ int kasan_populate_early_shadow(const void *shadow_start,
>  #ifndef kasan_mem_to_shadow
>  static inline void *kasan_mem_to_shadow(const void *addr)
>  {
> -       return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
> -               + KASAN_SHADOW_OFFSET;
> +       void *scaled;
> +
> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
> +               scaled = (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT);
> +       else
> +               scaled = (void *)((long)addr >> KASAN_SHADOW_SCALE_SHIFT);
> +
> +       return KASAN_SHADOW_OFFSET + scaled;
>  }
>  #endif
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 3fe77a360f1c..5766714872d3 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -645,15 +645,33 @@ void kasan_report_async(void)
>   */
>  void kasan_non_canonical_hook(unsigned long addr)
>  {
> +       unsigned long max_shadow_size = BIT(BITS_PER_LONG - KASAN_SHADOW_SCALE_SHIFT);
>         unsigned long orig_addr;
>         const char *bug_type;
>
>         /*
> -        * All addresses that came as a result of the memory-to-shadow mapping
> -        * (even for bogus pointers) must be >= KASAN_SHADOW_OFFSET.
> +        * With the default kasan_mem_to_shadow() algorithm, all addresses
> +        * returned by the memory-to-shadow mapping (even for bogus pointers)
> +        * must be within a certain displacement from KASAN_SHADOW_OFFSET.
> +        *
> +        * For Generic KASAN the displacement is unsigned so the mapping from zero
> +        * to the last kernel address needs checking.
> +        *
> +        * For Software Tag-Based KASAN, the displacement is signed, so
> +        * KASAN_SHADOW_OFFSET is the center of the range. Higher positive
> +        * addresses overflow, so the range that can't be part of
> +        * memory-to-shadow mapping is above the biggest positive address
> +        * mapping and below the lowest possible one.
>          */
> -       if (addr < KASAN_SHADOW_OFFSET)
> -               return;
> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
> +               if (addr < KASAN_SHADOW_OFFSET ||
> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
> +                       return;
> +       } else {
> +               if (addr < KASAN_SHADOW_OFFSET - max_shadow_size / 2 &&
> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size / 2)
> +                       return;

Ok, I think this would work for what I had in mind.

However, I just realized that this check is not entirely precise. When
doing the memory-to-shadow mapping, the memory address always has its
top byte set to 0xff: both the inlined compiler code and the outline
KASAN code do this. Thus, the possible values a shadow address can
take are the result of the memory-to-shadow mapping applied to
[0xff00000000000000, 0xffffffffffffffff], not to the whole address
space. So we can make this check more precise.

> +       }
>
>         orig_addr = (unsigned long)kasan_shadow_to_mem((void *)addr);
>
> diff --git a/scripts/gdb/linux/mm.py b/scripts/gdb/linux/mm.py
> index 7571aebbe650..2e63f3dedd53 100644
> --- a/scripts/gdb/linux/mm.py
> +++ b/scripts/gdb/linux/mm.py
> @@ -110,12 +110,13 @@ class aarch64_page_ops():
>          self.KERNEL_END = gdb.parse_and_eval("_end")
>
>          if constants.LX_CONFIG_KASAN_GENERIC or constants.LX_CONFIG_KASAN_SW_TAGS:
> +            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
>              if constants.LX_CONFIG_KASAN_GENERIC:
>                  self.KASAN_SHADOW_SCALE_SHIFT = 3
> +                self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
>              else:
>                  self.KASAN_SHADOW_SCALE_SHIFT = 4
> -            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
> -            self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
> +                self.KASAN_SHADOW_END = self.KASAN_SHADOW_OFFSET
>              self.PAGE_END = self.KASAN_SHADOW_END - (1 << (self.vabits_actual - self.KASAN_SHADOW_SCALE_SHIFT))
>          else:
>              self.PAGE_END = self._PAGE_END(self.VA_BITS_MIN)

We likely also need to update scripts/gdb/linux/kasan.py.

Also, later in the series, you change KASAN_SHADOW_OFFSET from a
config option into a runtime_const, which AFAIU would make these
scripts stop working.

Re: [PATCH v2 02/14] kasan: sw_tags: Check kasan_flag_enabled at runtime

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:16 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> From: Samuel Holland <samuel.holland@sifive.com>
>
> On RISC-V, the ISA extension required to dereference tagged pointers is
> optional, and the interface to enable pointer masking requires firmware
> support. Therefore, we must detect at runtime if sw_tags is usable on a
> given machine. Reuse the logic from hw_tags to dynamically enable KASAN.

Is this patch required on x86 as well? If so, I think it makes sense
to point it out here. And do the same in messages for other commits
that now mention RISC-V.


>
> This commit makes no functional change to the KASAN_HW_TAGS code path.
>
> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
>  include/linux/kasan-enabled.h | 15 +++++----------
>  mm/kasan/hw_tags.c            | 10 ----------
>  mm/kasan/tags.c               | 10 ++++++++++
>  3 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/kasan-enabled.h b/include/linux/kasan-enabled.h
> index 6f612d69ea0c..648bda9495b7 100644
> --- a/include/linux/kasan-enabled.h
> +++ b/include/linux/kasan-enabled.h
> @@ -4,7 +4,7 @@
>
>  #include <linux/static_key.h>
>
> -#ifdef CONFIG_KASAN_HW_TAGS
> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>
>  DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
>
> @@ -13,23 +13,18 @@ static __always_inline bool kasan_enabled(void)
>         return static_branch_likely(&kasan_flag_enabled);
>  }
>
> -static inline bool kasan_hw_tags_enabled(void)
> -{
> -       return kasan_enabled();
> -}
> -
> -#else /* CONFIG_KASAN_HW_TAGS */
> +#else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>
>  static inline bool kasan_enabled(void)
>  {
>         return IS_ENABLED(CONFIG_KASAN);
>  }
>
> +#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
> +
>  static inline bool kasan_hw_tags_enabled(void)
>  {
> -       return false;
> +       return IS_ENABLED(CONFIG_KASAN_HW_TAGS) && kasan_enabled();
>  }
>
> -#endif /* CONFIG_KASAN_HW_TAGS */
> -
>  #endif /* LINUX_KASAN_ENABLED_H */
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 9a6927394b54..7f82af13b6a6 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -45,13 +45,6 @@ static enum kasan_arg kasan_arg __ro_after_init;
>  static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
>  static enum kasan_arg_vmalloc kasan_arg_vmalloc __initdata;
>
> -/*
> - * Whether KASAN is enabled at all.
> - * The value remains false until KASAN is initialized by kasan_init_hw_tags().
> - */
> -DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
> -EXPORT_SYMBOL(kasan_flag_enabled);
> -
>  /*
>   * Whether the selected mode is synchronous, asynchronous, or asymmetric.
>   * Defaults to KASAN_MODE_SYNC.
> @@ -259,9 +252,6 @@ void __init kasan_init_hw_tags(void)
>
>         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",
>                 kasan_mode_info(),
>                 str_on_off(kasan_vmalloc_enabled()),
> diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> index d65d48b85f90..c111d98961ed 100644
> --- a/mm/kasan/tags.c
> +++ b/mm/kasan/tags.c
> @@ -32,6 +32,13 @@ enum kasan_arg_stacktrace {
>
>  static enum kasan_arg_stacktrace kasan_arg_stacktrace __initdata;
>
> +/*
> + * Whether KASAN is enabled at all.
> + * The value remains false until KASAN is initialized by kasan_init_tags().
> + */
> +DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
> +EXPORT_SYMBOL(kasan_flag_enabled);
> +
>  /* Whether to collect alloc/free stack traces. */
>  DEFINE_STATIC_KEY_TRUE(kasan_flag_stacktrace);
>
> @@ -92,6 +99,9 @@ void __init kasan_init_tags(void)
>                 if (WARN_ON(!stack_ring.entries))
>                         static_branch_disable(&kasan_flag_stacktrace);
>         }
> +
> +       /* KASAN is now initialized, enable it. */
> +       static_branch_enable(&kasan_flag_enabled);
>  }
>
>  static void save_stack_info(struct kmem_cache *cache, void *object,
> --
> 2.47.1
>

Re: [PATCH v2 03/14] kasan: sw_tags: Support outline stack tag generation

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:17 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> From: Samuel Holland <samuel.holland@sifive.com>
>
> This allows stack tagging to be disabled at runtime by tagging all
> stack objects with the match-all tag. This is necessary on RISC-V,
> where a kernel with KASAN_SW_TAGS enabled is expected to boot on
> hardware without pointer masking support.

Same question, is this needed on x86?



>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
>  mm/kasan/kasan.h   | 2 ++
>  mm/kasan/sw_tags.c | 9 +++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 129178be5e64..2fb26f74dff9 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -636,6 +636,8 @@ void *__asan_memset(void *addr, int c, ssize_t len);
>  void *__asan_memmove(void *dest, const void *src, ssize_t len);
>  void *__asan_memcpy(void *dest, const void *src, ssize_t len);
>
> +u8 __hwasan_generate_tag(void);
> +
>  void __hwasan_load1_noabort(void *);
>  void __hwasan_store1_noabort(void *);
>  void __hwasan_load2_noabort(void *);
> diff --git a/mm/kasan/sw_tags.c b/mm/kasan/sw_tags.c
> index b9382b5b6a37..94465a8a3640 100644
> --- a/mm/kasan/sw_tags.c
> +++ b/mm/kasan/sw_tags.c
> @@ -71,6 +71,15 @@ u8 kasan_random_tag(void)
>         return (u8)(state % (KASAN_TAG_MAX + 1));
>  }
>
> +u8 __hwasan_generate_tag(void)
> +{
> +       if (!kasan_enabled())
> +               return KASAN_TAG_KERNEL;
> +
> +       return kasan_random_tag();
> +}
> +EXPORT_SYMBOL(__hwasan_generate_tag);
> +
>  bool kasan_check_range(const void *addr, size_t size, bool write,
>                         unsigned long ret_ip)
>  {
> --
> 2.47.1
>

Re: [PATCH v2 06/14] x86: Add arch specific kasan functions

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:18 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> KASAN's software tag-based mode needs multiple macros/functions to
> handle tag and pointer interactions - mainly to set and retrieve tags
> from the top bits of a pointer.
>
> Mimic functions currently used by arm64 but change the tag's position to
> bits [60:57] in the pointer.
>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
>  arch/x86/include/asm/kasan.h | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> index de75306b932e..8829337a75fa 100644
> --- a/arch/x86/include/asm/kasan.h
> +++ b/arch/x86/include/asm/kasan.h
> @@ -3,6 +3,8 @@
>  #define _ASM_X86_KASAN_H
>
>  #include <linux/const.h>
> +#include <linux/kasan-tags.h>
> +#include <linux/types.h>
>  #define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>  #define KASAN_SHADOW_SCALE_SHIFT 3
>
> @@ -24,8 +26,33 @@
>                                                   KASAN_SHADOW_SCALE_SHIFT)))
>
>  #ifndef __ASSEMBLY__
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +
> +#define arch_kasan_set_tag(addr, tag)  __tag_set(addr, tag)

But __tag_set is defined below. I think these need to be reordered.

> +#define arch_kasan_reset_tag(addr)     __tag_reset(addr)
> +#define arch_kasan_get_tag(addr)       __tag_get(addr)
> +
> +#ifdef CONFIG_KASAN_SW_TAGS
> +
> +#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))
> +#else
> +#define __tag_shifted(tag)             0UL
> +#define __tag_reset(addr)              (addr)
> +#define __tag_get(addr)                        0
> +#endif /* CONFIG_KASAN_SW_TAGS */
>
>  #ifdef CONFIG_KASAN
> +
> +static inline const void *__tag_set(const void *addr, u8 tag)

A bit weird that __tag_set is defined under CONFIG_KASAN:
CONFIG_KASAN_SW_TAGS (or no condition, like on arm64) would make more
sense.


> +{
> +       u64 __addr = (u64)addr & ~__tag_shifted(KASAN_TAG_KERNEL);
> +       return (const void *)(__addr | __tag_shifted(tag));
> +}
> +
>  void __init kasan_early_init(void);
>  void __init kasan_init(void);
>  void __init kasan_populate_shadow_for_vaddr(void *va, size_t size, int nid);
> @@ -34,8 +61,9 @@ static inline void kasan_early_init(void) { }
>  static inline void kasan_init(void) { }
>  static inline void kasan_populate_shadow_for_vaddr(void *va, size_t size,
>                                                    int nid) { }
> -#endif
>
> -#endif
> +#endif /* CONFIG_KASAN */
> +
> +#endif /* __ASSEMBLY__ */
>
>  #endif
> --
> 2.47.1
>

Re: [PATCH v2 12/14] x86: Minimal SLAB alignment

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> Adjust x86 minimal SLAB alignment to match KASAN granularity size. In
> tag-based mode the size changes to 16 bytes so the value needs to be 4.

This 4 should be 16.

>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
>  arch/x86/include/asm/kasan.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> index 8829337a75fa..a75f0748a4b6 100644
> --- a/arch/x86/include/asm/kasan.h
> +++ b/arch/x86/include/asm/kasan.h
> @@ -36,6 +36,8 @@
>
>  #ifdef CONFIG_KASAN_SW_TAGS
>
> +#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT)

I believe ARCH_SLAB_MINALIGN needs to be defined in
include/asm/cache.h: at least other architectures have it there.


> +
>  #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))
> --
> 2.47.1
>

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> On x86, generic KASAN is setup in a way that needs a single
> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
> to facilitate boot time switching and it's a compiler ABI so it can't be
> changed during runtime.
>
> Software tag-based mode doesn't tie shadow start and end to any linear
> addresses as part of the compiler ABI so it can be changed during
> runtime.

KASAN_SHADOW_OFFSET is passed to the compiler via
hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
mode). So while we can change its value, it has to be known at compile
time. So I don't think using a runtime constant would work.

Which means that KASAN_SHADOW_OFFSET has to have such a value that
works for both 4 and 5 level page tables. This possibly means we might
need something different than the first patch in this series.

But in case I'm wrong, I left comments for the current code below.

> This notion, for KASAN purposes, allows to optimize out macros
> such us pgtable_l5_enabled() which would otherwise be used in every
> single KASAN related function.
>
> Use runtime_const infrastructure with pgtable_l5_enabled() to initialize
> the end address of KASAN's shadow address space. It's a good choice
> since in software tag based mode KASAN_SHADOW_OFFSET and
> KASAN_SHADOW_END refer to the same value and the offset in
> kasan_mem_to_shadow() is a signed negative value.
>
> Setup KASAN_SHADOW_END values so that they're aligned to 4TB in 4-level
> paging mode and to 2PB in 5-level paging mode. Also update x86 memory
> map documentation.
>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v2:
> - Change documentation kasan start address to non-dense values.
>
>  Documentation/arch/x86/x86_64/mm.rst |  6 ++++--
>  arch/x86/Kconfig                     |  3 +--
>  arch/x86/include/asm/kasan.h         | 14 +++++++++++++-
>  arch/x86/kernel/vmlinux.lds.S        |  1 +
>  arch/x86/mm/kasan_init_64.c          |  5 ++++-
>  5 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/arch/x86/x86_64/mm.rst b/Documentation/arch/x86/x86_64/mm.rst
> index f2db178b353f..5014ec322e19 100644
> --- a/Documentation/arch/x86/x86_64/mm.rst
> +++ b/Documentation/arch/x86/x86_64/mm.rst
> @@ -60,7 +60,8 @@ Complete virtual memory map with 4-level page tables
>     ffffe90000000000 |  -23    TB | ffffe9ffffffffff |    1 TB | ... unused hole
>     ffffea0000000000 |  -22    TB | ffffeaffffffffff |    1 TB | virtual memory map (vmemmap_base)
>     ffffeb0000000000 |  -21    TB | ffffebffffffffff |    1 TB | ... unused hole
> -   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory
> +   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory (generic mode)
> +   fffff40000000000 |   -8    TB | fffffc0000000000 |    8 TB | KASAN shadow memory (software tag-based mode)
>    __________________|____________|__________________|_________|____________________________________________________________
>                                                                |
>                                                                | Identical layout to the 56-bit one from here on:
> @@ -130,7 +131,8 @@ Complete virtual memory map with 5-level page tables
>     ffd2000000000000 |  -11.5  PB | ffd3ffffffffffff |  0.5 PB | ... unused hole
>     ffd4000000000000 |  -11    PB | ffd5ffffffffffff |  0.5 PB | virtual memory map (vmemmap_base)
>     ffd6000000000000 |  -10.5  PB | ffdeffffffffffff | 2.25 PB | ... unused hole
> -   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory
> +   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory (generic mode)
> +   ffe0000000000000 |   -6    PB | fff0000000000000 |    4 PB | KASAN shadow memory (software tag-based mode)
>    __________________|____________|__________________|_________|____________________________________________________________
>                                                                |
>                                                                | Identical layout to the 47-bit one from here on:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 6df7779ed6da..f4ef64bf824a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -400,8 +400,7 @@ config AUDIT_ARCH
>
>  config KASAN_SHADOW_OFFSET
>         hex
> -       depends on KASAN
> -       default 0xdffffc0000000000
> +       default 0xdffffc0000000000 if KASAN_GENERIC

Let's put a comment here explaining what happens if !KASAN_GENERIC.

Also, as I mentioned in the first patch, we need to figure out what to
do with scripts/gdb/linux/kasan.py.

>
>  config HAVE_INTEL_TXT
>         def_bool y
> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> index a75f0748a4b6..4bfd3641af84 100644
> --- a/arch/x86/include/asm/kasan.h
> +++ b/arch/x86/include/asm/kasan.h
> @@ -5,7 +5,7 @@
>  #include <linux/const.h>
>  #include <linux/kasan-tags.h>
>  #include <linux/types.h>
> -#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
> +
>  #define KASAN_SHADOW_SCALE_SHIFT 3
>
>  /*
> @@ -14,6 +14,8 @@
>   * for kernel really starts from compiler's shadow offset +
>   * 'kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT
>   */
> +#ifdef CONFIG_KASAN_GENERIC
> +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>  #define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
>                                         ((-1UL << __VIRTUAL_MASK_SHIFT) >> \
>                                                 KASAN_SHADOW_SCALE_SHIFT))
> @@ -24,12 +26,22 @@
>  #define KASAN_SHADOW_END        (KASAN_SHADOW_START + \
>                                         (1ULL << (__VIRTUAL_MASK_SHIFT - \
>                                                   KASAN_SHADOW_SCALE_SHIFT)))
> +#endif
> +
>
>  #ifndef __ASSEMBLY__
> +#include <asm/runtime-const.h>
>  #include <linux/bitops.h>
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
>
> +#ifdef CONFIG_KASAN_SW_TAGS
> +extern unsigned long KASAN_SHADOW_END_RC;
> +#define KASAN_SHADOW_END       runtime_const_ptr(KASAN_SHADOW_END_RC)
> +#define KASAN_SHADOW_OFFSET    KASAN_SHADOW_END
> +#define KASAN_SHADOW_START     (KASAN_SHADOW_END - ((UL(1)) << (__VIRTUAL_MASK_SHIFT - KASAN_SHADOW_SCALE_SHIFT)))

Any reason these are under __ASSEMBLY__? They seem to belong better
together with the CONFIG_KASAN_GENERIC definitions above.

> +#endif
> +
>  #define arch_kasan_set_tag(addr, tag)  __tag_set(addr, tag)
>  #define arch_kasan_reset_tag(addr)     __tag_reset(addr)
>  #define arch_kasan_get_tag(addr)       __tag_get(addr)
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 0deb4887d6e9..df6c85f8f48f 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -353,6 +353,7 @@ SECTIONS
>
>         RUNTIME_CONST_VARIABLES
>         RUNTIME_CONST(ptr, USER_PTR_MAX)
> +       RUNTIME_CONST(ptr, KASAN_SHADOW_END_RC)
>
>         . = ALIGN(PAGE_SIZE);
>
> diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
> index 299a2144dac4..5ca5862a5cd6 100644
> --- a/arch/x86/mm/kasan_init_64.c
> +++ b/arch/x86/mm/kasan_init_64.c
> @@ -358,6 +358,9 @@ void __init kasan_init(void)
>         int i;
>
>         memcpy(early_top_pgt, init_top_pgt, sizeof(early_top_pgt));
> +       unsigned long KASAN_SHADOW_END_RC = pgtable_l5_enabled() ? 0xfff0000000000000 : 0xfffffc0000000000;

I think defining these constants in arch/x86/include/asm/kasan.h is
cleaner than hardcoding them here.







> +
> +       runtime_const_init(ptr, KASAN_SHADOW_END_RC);
>
>         /*
>          * We use the same shadow offset for 4- and 5-level paging to
> @@ -372,7 +375,7 @@ void __init kasan_init(void)
>          * bunch of things like kernel code, modules, EFI mapping, etc.
>          * We need to take extra steps to not overwrite them.
>          */
> -       if (pgtable_l5_enabled()) {
> +       if (pgtable_l5_enabled() && !IS_ENABLED(CONFIG_KASAN_SW_TAGS)) {
>                 void *ptr;
>
>                 ptr = (void *)pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_END));
> --
> 2.47.1
>

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[Andrey Konovalov]

On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> Make CONFIG_KASAN_SW_TAGS available for x86 machines if they have
> ADDRESS_MASKING enabled (LAM) as that works similarly to Top-Byte Ignore
> (TBI) that allows the software tag-based mode on arm64 platform.
>
> Set scale macro based on KASAN mode: in software tag-based mode 32 bytes
> of memory map to one shadow byte and 16 in generic mode.

These should be 16 and 8.

>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v2:
> - Remove KASAN dense code.
>
>  arch/x86/Kconfig                | 6 ++++++
>  arch/x86/boot/compressed/misc.h | 1 +
>  arch/x86/include/asm/kasan.h    | 2 +-
>  arch/x86/kernel/setup.c         | 2 ++
>  4 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f4ef64bf824a..dc48eb5b664f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -195,6 +195,7 @@ config X86
>         select HAVE_ARCH_JUMP_LABEL_RELATIVE
>         select HAVE_ARCH_KASAN                  if X86_64
>         select HAVE_ARCH_KASAN_VMALLOC          if X86_64
> +       select HAVE_ARCH_KASAN_SW_TAGS          if ADDRESS_MASKING
>         select HAVE_ARCH_KFENCE
>         select HAVE_ARCH_KMSAN                  if X86_64
>         select HAVE_ARCH_KGDB
> @@ -402,6 +403,11 @@ config KASAN_SHADOW_OFFSET
>         hex
>         default 0xdffffc0000000000 if KASAN_GENERIC
>
> +config KASAN_SHADOW_SCALE_SHIFT
> +       int
> +       default 4 if KASAN_SW_TAGS
> +       default 3

What's the purpose of this config option? I think we can just change
the value of the KASAN_SHADOW_SCALE_SHIFT define when KASAN_SW_TAGS is
enabled.


> +
>  config HAVE_INTEL_TXT
>         def_bool y
>         depends on INTEL_IOMMU && ACPI
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index dd8d1a85f671..f6a87e9ad200 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -13,6 +13,7 @@
>  #undef CONFIG_PARAVIRT_SPINLOCKS
>  #undef CONFIG_KASAN
>  #undef CONFIG_KASAN_GENERIC
> +#undef CONFIG_KASAN_SW_TAGS
>
>  #define __NO_FORTIFY
>
> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> index 4bfd3641af84..cfc31e4a2f70 100644
> --- a/arch/x86/include/asm/kasan.h
> +++ b/arch/x86/include/asm/kasan.h
> @@ -6,7 +6,7 @@
>  #include <linux/kasan-tags.h>
>  #include <linux/types.h>
>
> -#define KASAN_SHADOW_SCALE_SHIFT 3
> +#define KASAN_SHADOW_SCALE_SHIFT CONFIG_KASAN_SHADOW_SCALE_SHIFT
>
>  /*
>   * Compiler uses shadow offset assuming that addresses start
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index cebee310e200..768990c573ea 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1124,6 +1124,8 @@ void __init setup_arch(char **cmdline_p)
>
>         kasan_init();
>
> +       kasan_init_sw_tags();
> +
>         /*
>          * Sync back kernel address range.
>          *
> --
> 2.47.1
>

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[kernel test robot]

Hello,

by this commit, we noticed below config diff with its parent
(
* 3742b7b32f28b x86: Make software tag-based kasan available
* 0ef701bc87cdd x86: runtime_const used for KASAN_SHADOW_END  <-- parent
)

@@ -293,7 +293,7 @@ CONFIG_ARCH_HAS_CPU_RELAX=y
 CONFIG_ARCH_HIBERNATION_POSSIBLE=y
 CONFIG_ARCH_SUSPEND_POSSIBLE=y
 CONFIG_AUDIT_ARCH=y
-CONFIG_KASAN_SHADOW_OFFSET=0xdffffc0000000000
+CONFIG_KASAN_SHADOW_SCALE_SHIFT=4
 CONFIG_ARCH_SUPPORTS_UPROBES=y
 CONFIG_FIX_EARLYCON_MEM=y
 CONFIG_PGTABLE_LEVELS=5
@@ -5387,13 +5387,15 @@ CONFIG_DEBUG_KMAP_LOCAL=y
 CONFIG_ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP=y
 CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP=y
 CONFIG_HAVE_ARCH_KASAN=y
+CONFIG_HAVE_ARCH_KASAN_SW_TAGS=y
 CONFIG_HAVE_ARCH_KASAN_VMALLOC=y
 CONFIG_CC_HAS_KASAN_GENERIC=y
 CONFIG_CC_HAS_KASAN_SW_TAGS=y
 CONFIG_CC_HAS_WORKING_NOSANITIZE_ADDRESS=y
 CONFIG_KASAN=y
 CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y
-CONFIG_KASAN_GENERIC=y
+# CONFIG_KASAN_GENERIC is not set
+CONFIG_KASAN_SW_TAGS=y
 # CONFIG_KASAN_OUTLINE is not set
 CONFIG_KASAN_INLINE=y
 # CONFIG_KASAN_STACK is not set


below full report FYI.


kernel test robot noticed "Oops:general_protection_fault,probably_for_non-canonical_address#:#[##]PREEMPT_KASAN" on:

commit: 3742b7b32f28b574e97da7c4f50593877b99e95c ("[PATCH v2 14/14] x86: Make software tag-based kasan available")
url: https://github.com/intel-lab-lkp/linux/commits/Maciej-Wieczor-Retman/kasan-sw_tags-Use-arithmetic-shift-for-shadow-computation/20250218-162135
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 882b86fd4e0d49bf91148dbadcdbece19ded40e6
patch link: https://lore.kernel.org/all/d266338a0eae1f673802e41d7230c4c92c3532b3.1739866028.git.maciej.wieczor-retman@intel.com/
patch subject: [PATCH v2 14/14] x86: Make software tag-based kasan available

in testcase: boot

config: x86_64-randconfig-161-20250219
compiler: clang-19
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+--------------------------------------------------------------------------------------+------------+------------+
|                                                                                      | 0ef701bc87 | 3742b7b32f |
+--------------------------------------------------------------------------------------+------------+------------+
| boot_successes                                                                       | 21         | 0          |
| boot_failures                                                                        | 0          | 18         |
| Oops:general_protection_fault,probably_for_non-canonical_address#:#[##]PREEMPT_KASAN | 0          | 17         |
| RIP:stack_depot_save_flags                                                           | 0          | 17         |
| Kernel_panic-not_syncing:Fatal_exception                                             | 0          | 17         |
| KASAN:maybe_wild-memory-access_in_range[#-#]                                         | 0          | 2          |
+--------------------------------------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202502201048.208452a-lkp@intel.com


[   11.050670][    T0] Oops: general protection fault, probably for non-canonical address 0xfbff888100044018: 0000 [#1] PREEMPT KASAN
[   11.050681][    T0] KASAN: maybe wild-memory-access in range [0xbff8c81000440180-0xbff8c8100044018f]
[   11.050690][    T0] CPU: 0 UID: 0 PID: 0 Comm: swapper Tainted: G                T  6.14.0-rc2-00036-g3742b7b32f28 #2
[   11.050700][    T0] Tainted: [T]=RANDSTRUCT
[ 11.050704][ T0] RIP: 0010:stack_depot_save_flags (kbuild/src/smatch/lib/stackdepot.c:335) 
[ 11.050720][ T0] Code: 85 48 85 c0 0f 84 cf 01 00 00 48 8b 0d 35 b0 35 02 4c 8d 24 08 41 81 e7 ff ff 01 00 89 ca c1 e2 0d 81 e2 00 00 fe 07 44 09 fa <89> 54 08 18 4c 89 24 08 4c 89 64 08 08 48 03 4d c0 48 89 0d 04 b0
All code
========
   0:	85 48 85             	test   %ecx,-0x7b(%rax)
   3:	c0 0f 84             	rorb   $0x84,(%rdi)
   6:	cf                   	iret
   7:	01 00                	add    %eax,(%rax)
   9:	00 48 8b             	add    %cl,-0x75(%rax)
   c:	0d 35 b0 35 02       	or     $0x235b035,%eax
  11:	4c 8d 24 08          	lea    (%rax,%rcx,1),%r12
  15:	41 81 e7 ff ff 01 00 	and    $0x1ffff,%r15d
  1c:	89 ca                	mov    %ecx,%edx
  1e:	c1 e2 0d             	shl    $0xd,%edx
  21:	81 e2 00 00 fe 07    	and    $0x7fe0000,%edx
  27:	44 09 fa             	or     %r15d,%edx
  2a:*	89 54 08 18          	mov    %edx,0x18(%rax,%rcx,1)		<-- trapping instruction
  2e:	4c 89 24 08          	mov    %r12,(%rax,%rcx,1)
  32:	4c 89 64 08 08       	mov    %r12,0x8(%rax,%rcx,1)
  37:	48 03 4d c0          	add    -0x40(%rbp),%rcx
  3b:	48                   	rex.W
  3c:	89                   	.byte 0x89
  3d:	0d                   	.byte 0xd
  3e:	04 b0                	add    $0xb0,%al

Code starting with the faulting instruction
===========================================
   0:	89 54 08 18          	mov    %edx,0x18(%rax,%rcx,1)
   4:	4c 89 24 08          	mov    %r12,(%rax,%rcx,1)
   8:	4c 89 64 08 08       	mov    %r12,0x8(%rax,%rcx,1)
   d:	48 03 4d c0          	add    -0x40(%rbp),%rcx
  11:	48                   	rex.W
  12:	89                   	.byte 0x89
  13:	0d                   	.byte 0xd
  14:	04 b0                	add    $0xb0,%al
[   11.050728][    T0] RSP: 0000:ffffffff84207b08 EFLAGS: 00010002
[   11.050735][    T0] RAX: fbff888100044000 RBX: 0000000000000000 RCX: 0000000000000000
[   11.050765][    T0] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000000
[   11.050770][    T0] RBP: ffffffff84207b60 R08: ffff8883ee081360 R09: fbff888100044000
[   11.050774][    T0] R10: 0000000000000000 R11: 0000000000000000 R12: fbff888100044000
[   11.050779][    T0] R13: 00000000f44e9436 R14: ffffffff84207b70 R15: 0000000000000001
[   11.050784][    T0] FS:  0000000000000000(0000) GS:ffffffff842fc000(0000) knlGS:0000000000000000
[   11.050791][    T0] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   11.050796][    T0] CR2: ffff88843ffff000 CR3: 00000000042cd000 CR4: 00000000000000b0
[   11.050804][    T0] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   11.050808][    T0] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   11.050813][    T0] Call Trace:
[   11.050816][    T0]  <TASK>
[ 11.050821][ T0] ? __die_body (kbuild/src/smatch/arch/x86/kernel/dumpstack.c:421) 
[ 11.050831][ T0] ? die_addr (kbuild/src/smatch/arch/x86/kernel/dumpstack.c:?) 
[ 11.050838][ T0] ? exc_general_protection (kbuild/src/smatch/arch/x86/kernel/traps.c:789) 
[ 11.050862][ T0] ? asm_exc_general_protection (kbuild/src/smatch/arch/x86/include/asm/idtentry.h:617) 


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250220/202502201048.208452a-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[Andrey Konovalov]

On Thu, Feb 20, 2025 at 12:31 AM Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
> <maciej.wieczor-retman@intel.com> wrote:
> >
> > Make CONFIG_KASAN_SW_TAGS available for x86 machines if they have
> > ADDRESS_MASKING enabled (LAM) as that works similarly to Top-Byte Ignore
> > (TBI) that allows the software tag-based mode on arm64 platform.
> >
> > Set scale macro based on KASAN mode: in software tag-based mode 32 bytes
> > of memory map to one shadow byte and 16 in generic mode.
>
> These should be 16 and 8.
>
> >
> > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> > ---
> > Changelog v2:
> > - Remove KASAN dense code.
> >
> >  arch/x86/Kconfig                | 6 ++++++
> >  arch/x86/boot/compressed/misc.h | 1 +
> >  arch/x86/include/asm/kasan.h    | 2 +-
> >  arch/x86/kernel/setup.c         | 2 ++
> >  4 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index f4ef64bf824a..dc48eb5b664f 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -195,6 +195,7 @@ config X86
> >         select HAVE_ARCH_JUMP_LABEL_RELATIVE
> >         select HAVE_ARCH_KASAN                  if X86_64
> >         select HAVE_ARCH_KASAN_VMALLOC          if X86_64
> > +       select HAVE_ARCH_KASAN_SW_TAGS          if ADDRESS_MASKING
> >         select HAVE_ARCH_KFENCE
> >         select HAVE_ARCH_KMSAN                  if X86_64
> >         select HAVE_ARCH_KGDB
> > @@ -402,6 +403,11 @@ config KASAN_SHADOW_OFFSET
> >         hex
> >         default 0xdffffc0000000000 if KASAN_GENERIC
> >
> > +config KASAN_SHADOW_SCALE_SHIFT
> > +       int
> > +       default 4 if KASAN_SW_TAGS
> > +       default 3
>
> What's the purpose of this config option? I think we can just change
> the value of the KASAN_SHADOW_SCALE_SHIFT define when KASAN_SW_TAGS is
> enabled.
>
>
> > +
> >  config HAVE_INTEL_TXT
> >         def_bool y
> >         depends on INTEL_IOMMU && ACPI
> > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> > index dd8d1a85f671..f6a87e9ad200 100644
> > --- a/arch/x86/boot/compressed/misc.h
> > +++ b/arch/x86/boot/compressed/misc.h
> > @@ -13,6 +13,7 @@
> >  #undef CONFIG_PARAVIRT_SPINLOCKS
> >  #undef CONFIG_KASAN
> >  #undef CONFIG_KASAN_GENERIC
> > +#undef CONFIG_KASAN_SW_TAGS
> >
> >  #define __NO_FORTIFY
> >
> > diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
> > index 4bfd3641af84..cfc31e4a2f70 100644
> > --- a/arch/x86/include/asm/kasan.h
> > +++ b/arch/x86/include/asm/kasan.h
> > @@ -6,7 +6,7 @@
> >  #include <linux/kasan-tags.h>
> >  #include <linux/types.h>
> >
> > -#define KASAN_SHADOW_SCALE_SHIFT 3
> > +#define KASAN_SHADOW_SCALE_SHIFT CONFIG_KASAN_SHADOW_SCALE_SHIFT
> >
> >  /*
> >   * Compiler uses shadow offset assuming that addresses start
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index cebee310e200..768990c573ea 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -1124,6 +1124,8 @@ void __init setup_arch(char **cmdline_p)
> >
> >         kasan_init();
> >
> > +       kasan_init_sw_tags();
> > +
> >         /*
> >          * Sync back kernel address range.
> >          *
> > --
> > 2.47.1
> >

Also please update the descriptions of all related options in lib/Kconfig.kasan.

Re: [PATCH v2 12/14] x86: Minimal SLAB alignment

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:30:48 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> Adjust x86 minimal SLAB alignment to match KASAN granularity size. In
>> tag-based mode the size changes to 16 bytes so the value needs to be 4.
>
>This 4 should be 16.

Thanks!

>
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>>  arch/x86/include/asm/kasan.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
>> index 8829337a75fa..a75f0748a4b6 100644
>> --- a/arch/x86/include/asm/kasan.h
>> +++ b/arch/x86/include/asm/kasan.h
>> @@ -36,6 +36,8 @@
>>
>>  #ifdef CONFIG_KASAN_SW_TAGS
>>
>> +#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT)
>
>I believe ARCH_SLAB_MINALIGN needs to be defined in
>include/asm/cache.h: at least other architectures have it there.

Okay, I'll correct it.

>
>
>> +
>>  #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))
>> --
>> 2.47.1
>>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 06/14] x86: Add arch specific kasan functions

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:30:34 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:18 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> KASAN's software tag-based mode needs multiple macros/functions to
>> handle tag and pointer interactions - mainly to set and retrieve tags
>> from the top bits of a pointer.
>>
>> Mimic functions currently used by arm64 but change the tag's position to
>> bits [60:57] in the pointer.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>>  arch/x86/include/asm/kasan.h | 32 ++++++++++++++++++++++++++++++--
>>  1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
>> index de75306b932e..8829337a75fa 100644
>> --- a/arch/x86/include/asm/kasan.h
>> +++ b/arch/x86/include/asm/kasan.h
>> @@ -3,6 +3,8 @@
>>  #define _ASM_X86_KASAN_H
>>
>>  #include <linux/const.h>
>> +#include <linux/kasan-tags.h>
>> +#include <linux/types.h>
>>  #define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>>  #define KASAN_SHADOW_SCALE_SHIFT 3
>>
>> @@ -24,8 +26,33 @@
>>                                                   KASAN_SHADOW_SCALE_SHIFT)))
>>
>>  #ifndef __ASSEMBLY__
>> +#include <linux/bitops.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>> +
>> +#define arch_kasan_set_tag(addr, tag)  __tag_set(addr, tag)
>
>But __tag_set is defined below. I think these need to be reordered.

Oh, right. I'll fix it.

>
>> +#define arch_kasan_reset_tag(addr)     __tag_reset(addr)
>> +#define arch_kasan_get_tag(addr)       __tag_get(addr)
>> +
>> +#ifdef CONFIG_KASAN_SW_TAGS
>> +
>> +#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))
>> +#else
>> +#define __tag_shifted(tag)             0UL
>> +#define __tag_reset(addr)              (addr)
>> +#define __tag_get(addr)                        0
>> +#endif /* CONFIG_KASAN_SW_TAGS */
>>
>>  #ifdef CONFIG_KASAN
>> +
>> +static inline const void *__tag_set(const void *addr, u8 tag)
>
>A bit weird that __tag_set is defined under CONFIG_KASAN:
>CONFIG_KASAN_SW_TAGS (or no condition, like on arm64) would make more
>sense.

Ah sorry, I misread the arm code. I'll try doing a no condition.

>
>> +{
>> +       u64 __addr = (u64)addr & ~__tag_shifted(KASAN_TAG_KERNEL);
>> +       return (const void *)(__addr | __tag_shifted(tag));
>> +}
>> +
>>  void __init kasan_early_init(void);
>>  void __init kasan_init(void);
>>  void __init kasan_populate_shadow_for_vaddr(void *va, size_t size, int nid);
>> @@ -34,8 +61,9 @@ static inline void kasan_early_init(void) { }
>>  static inline void kasan_init(void) { }
>>  static inline void kasan_populate_shadow_for_vaddr(void *va, size_t size,
>>                                                    int nid) { }
>> -#endif
>>
>> -#endif
>> +#endif /* CONFIG_KASAN */
>> +
>> +#endif /* __ASSEMBLY__ */
>>
>>  #endif
>> --
>> 2.47.1
>>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:29:14 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:16 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> From: Samuel Holland <samuel.holland@sifive.com>
>>
>> Currently, kasan_mem_to_shadow() uses a logical right shift, which turns
>> canonical kernel addresses into non-canonical addresses by clearing the
>> high KASAN_SHADOW_SCALE_SHIFT bits. The value of KASAN_SHADOW_OFFSET is
>> then chosen so that the addition results in a canonical address for the
>> shadow memory.
>>
>> For KASAN_GENERIC, this shift/add combination is ABI with the compiler,
>> because KASAN_SHADOW_OFFSET is used in compiler-generated inline tag
>> checks[1], which must only attempt to dereference canonical addresses.
>>
>> However, for KASAN_SW_TAGS we have some freedom to change the algorithm
>> without breaking the ABI. Because TBI is enabled for kernel addresses,
>> the top bits of shadow memory addresses computed during tag checks are
>> irrelevant, and so likewise are the top bits of KASAN_SHADOW_OFFSET.
>> This is demonstrated by the fact that LLVM uses a logical right shift
>> in the tag check fast path[2] but a sbfx (signed bitfield extract)
>> instruction in the slow path[3] without causing any issues.
>>
>> Using an arithmetic shift in kasan_mem_to_shadow() provides a number of
>> benefits:
>>
>> 1) The memory layout is easier to understand. KASAN_SHADOW_OFFSET
>> becomes a canonical memory address, and the shifted pointer becomes a
>> negative offset, so KASAN_SHADOW_OFFSET == KASAN_SHADOW_END regardless
>> of the shift amount or the size of the virtual address space.
>>
>> 2) KASAN_SHADOW_OFFSET becomes a simpler constant, requiring only one
>> instruction to load instead of two. Since it must be loaded in each
>> function with a tag check, this decreases kernel text size by 0.5%.
>>
>> 3) This shift and the sign extension from kasan_reset_tag() can be
>> combined into a single sbfx instruction. When this same algorithm change
>> is applied to the compiler, it removes an instruction from each inline
>> tag check, further reducing kernel text size by an additional 4.6%.
>>
>> These benefits extend to other architectures as well. On RISC-V, where
>> the baseline ISA does not shifted addition or have an equivalent to the
>> sbfx instruction, loading KASAN_SHADOW_OFFSET is reduced from 3 to 2
>> instructions, and kasan_mem_to_shadow(kasan_reset_tag(addr)) similarly
>> combines two consecutive right shifts.
>>
>> Due to signed memory-to-shadow mapping kasan_non_canonical_hook() needs
>> changes - specifically the first part that tries to deduce if a faulty
>> address came from kasan_mem_to_shadow(). Previous value of
>> KASAN_SHADOW_OFFSET prevented any overflows when trying to map the
>> entire linear address space to shadow memory so the check in
>> kasan_non_canonical_hook() could consist of only checking whether the
>> address isn't below KASAN_SHADOW_OFFSET.
>>
>> The signed memory-to-shadow conversion means negative addresses will be
>> mapped below KASAN_SHADOW_OFFSET and positive addresses will map above
>> KASAN_SHADOW_OFFSET. When looking at the mapping of the entire address
>> space there will be an overflow when a big enough positive address will
>> be passed to kasan_mem_to_shadow(). Then the question of finding
>> addresses that couldn't come from kasan_mem_to_shadow() can be reduced
>> to figuring out if the address isn't above the highest overflowed value
>> (most positive address possible) AND below the most negative address
>> possible.
>
>Is there any reason we need this change for x86 SW_TAGS besides the
>optimization benefits?

I wanted to have the shadow memory boundries aligned properly, to not waste page
table entries, so the memory map is more straight forward. This patch helps with
that, I don't think it would have worked without it.

>
>Is it required for the "x86: runtime_const used for KASAN_SHADOW_END"
>patch? If so, please check my comment there first.

Probably not, even if the shadow memory boundry addresses are static defines
it'd be nice to have this patch to already think about the mappings as signed
and about offset as the end of the shadow memory space.

I'll think if I can keep all the things mentioned above without runtime_const. I
had some idea with non-canonical offsets but I need to write this out a bit more
yet.

>
>>
>> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L1316 [1]
>> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L895 [2]
>> Link: https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp#L669 [3]
>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v2: (Maciej)
>> - Correct address range that's checked in kasan_non_canonical_hook().
>>   Adjust the comment inside.
>> - Remove part of comment from arch/arm64/include/asm/memory.h.
>> - Append patch message paragraph about the overflow in
>>   kasan_non_canonical_hook().
>>
>>  arch/arm64/Kconfig              | 10 +++++-----
>>  arch/arm64/include/asm/memory.h | 14 +++++++++++++-
>>  arch/arm64/mm/kasan_init.c      |  7 +++++--
>>  include/linux/kasan.h           | 10 ++++++++--
>>  mm/kasan/report.c               | 26 ++++++++++++++++++++++----
>>  scripts/gdb/linux/mm.py         |  5 +++--
>>  6 files changed, 56 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index fcdd0ed3eca8..fe7d79b447c3 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -426,11 +426,11 @@ config KASAN_SHADOW_OFFSET
>>         default 0xdffffe0000000000 if ARM64_VA_BITS_42 && !KASAN_SW_TAGS
>>         default 0xdfffffc000000000 if ARM64_VA_BITS_39 && !KASAN_SW_TAGS
>>         default 0xdffffff800000000 if ARM64_VA_BITS_36 && !KASAN_SW_TAGS
>> -       default 0xefff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
>> -       default 0xefffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
>> -       default 0xeffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
>> -       default 0xefffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
>> -       default 0xeffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
>> +       default 0xffff800000000000 if (ARM64_VA_BITS_48 || (ARM64_VA_BITS_52 && !ARM64_16K_PAGES)) && KASAN_SW_TAGS
>> +       default 0xffffc00000000000 if (ARM64_VA_BITS_47 || ARM64_VA_BITS_52) && ARM64_16K_PAGES && KASAN_SW_TAGS
>> +       default 0xfffffe0000000000 if ARM64_VA_BITS_42 && KASAN_SW_TAGS
>> +       default 0xffffffc000000000 if ARM64_VA_BITS_39 && KASAN_SW_TAGS
>> +       default 0xfffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
>
>Ah, we also need to update Documentation/arch/arm64/kasan-offsets.sh,
>these offsets are generated by that script.
>
>Let's also point out in the commit message, that this change does not
>move the location of the shadow memory but only changes the way that
>location is calculated.

Will do, thanks :)

>
>>         default 0xffffffffffffffff
>>
>>  config UNWIND_TABLES
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index 717829df294e..e71cdf036287 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -89,7 +89,15 @@
>>   *
>>   * KASAN_SHADOW_END is defined first as the shadow address that corresponds to
>>   * the upper bound of possible virtual kernel memory addresses UL(1) << 64
>> - * according to the mapping formula.
>> + * according to the mapping formula. For Generic KASAN, the address in the
>> + * mapping formula is treated as unsigned (part of the compiler's ABI), so the
>> + * end of the shadow memory region is at a large positive offset from
>> + * KASAN_SHADOW_OFFSET. For Software Tag-Based KASAN, the address in the
>> + * formula is treated as signed. Since all kernel addresses are negative, they
>> + * map to shadow memory below KASAN_SHADOW_OFFSET, making KASAN_SHADOW_OFFSET
>> + * itself the end of the shadow memory region. (User pointers are positive and
>> + * would map to shadow memory above KASAN_SHADOW_OFFSET, but shadow memory is
>> + * not allocated for them.)
>>   *
>>   * KASAN_SHADOW_START is defined second based on KASAN_SHADOW_END. The shadow
>>   * memory start must map to the lowest possible kernel virtual memory address
>> @@ -100,7 +108,11 @@
>>   */
>>  #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
>>  #define KASAN_SHADOW_OFFSET    _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>> +#ifdef CONFIG_KASAN_GENERIC
>>  #define KASAN_SHADOW_END       ((UL(1) << (64 - KASAN_SHADOW_SCALE_SHIFT)) + KASAN_SHADOW_OFFSET)
>> +#else
>> +#define KASAN_SHADOW_END       KASAN_SHADOW_OFFSET
>> +#endif
>>  #define _KASAN_SHADOW_START(va)        (KASAN_SHADOW_END - (UL(1) << ((va) - KASAN_SHADOW_SCALE_SHIFT)))
>>  #define KASAN_SHADOW_START     _KASAN_SHADOW_START(vabits_actual)
>>  #define PAGE_END               KASAN_SHADOW_START
>> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
>> index b65a29440a0c..6836e571555c 100644
>> --- a/arch/arm64/mm/kasan_init.c
>> +++ b/arch/arm64/mm/kasan_init.c
>> @@ -198,8 +198,11 @@ static bool __init root_level_aligned(u64 addr)
>>  /* The early shadow maps everything to a single page of zeroes */
>>  asmlinkage void __init kasan_early_init(void)
>>  {
>> -       BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
>> -               KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
>> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
>> +               BUILD_BUG_ON(KASAN_SHADOW_OFFSET !=
>> +                       KASAN_SHADOW_END - (1UL << (64 - KASAN_SHADOW_SCALE_SHIFT)));
>> +       else
>> +               BUILD_BUG_ON(KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
>>         BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS), SHADOW_ALIGN));
>>         BUILD_BUG_ON(!IS_ALIGNED(_KASAN_SHADOW_START(VA_BITS_MIN), SHADOW_ALIGN));
>>         BUILD_BUG_ON(!IS_ALIGNED(KASAN_SHADOW_END, SHADOW_ALIGN));
>> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
>> index 890011071f2b..b396feca714f 100644
>> --- a/include/linux/kasan.h
>> +++ b/include/linux/kasan.h
>> @@ -61,8 +61,14 @@ int kasan_populate_early_shadow(const void *shadow_start,
>>  #ifndef kasan_mem_to_shadow
>>  static inline void *kasan_mem_to_shadow(const void *addr)
>>  {
>> -       return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
>> -               + KASAN_SHADOW_OFFSET;
>> +       void *scaled;
>> +
>> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
>> +               scaled = (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT);
>> +       else
>> +               scaled = (void *)((long)addr >> KASAN_SHADOW_SCALE_SHIFT);
>> +
>> +       return KASAN_SHADOW_OFFSET + scaled;
>>  }
>>  #endif
>>
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index 3fe77a360f1c..5766714872d3 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -645,15 +645,33 @@ void kasan_report_async(void)
>>   */
>>  void kasan_non_canonical_hook(unsigned long addr)
>>  {
>> +       unsigned long max_shadow_size = BIT(BITS_PER_LONG - KASAN_SHADOW_SCALE_SHIFT);
>>         unsigned long orig_addr;
>>         const char *bug_type;
>>
>>         /*
>> -        * All addresses that came as a result of the memory-to-shadow mapping
>> -        * (even for bogus pointers) must be >= KASAN_SHADOW_OFFSET.
>> +        * With the default kasan_mem_to_shadow() algorithm, all addresses
>> +        * returned by the memory-to-shadow mapping (even for bogus pointers)
>> +        * must be within a certain displacement from KASAN_SHADOW_OFFSET.
>> +        *
>> +        * For Generic KASAN the displacement is unsigned so the mapping from zero
>> +        * to the last kernel address needs checking.
>> +        *
>> +        * For Software Tag-Based KASAN, the displacement is signed, so
>> +        * KASAN_SHADOW_OFFSET is the center of the range. Higher positive
>> +        * addresses overflow, so the range that can't be part of
>> +        * memory-to-shadow mapping is above the biggest positive address
>> +        * mapping and below the lowest possible one.
>>          */
>> -       if (addr < KASAN_SHADOW_OFFSET)
>> -               return;
>> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
>> +               if (addr < KASAN_SHADOW_OFFSET ||
>> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
>> +                       return;
>> +       } else {
>> +               if (addr < KASAN_SHADOW_OFFSET - max_shadow_size / 2 &&
>> +                   addr >= KASAN_SHADOW_OFFSET + max_shadow_size / 2)
>> +                       return;
>
>Ok, I think this would work for what I had in mind.
>
>However, I just realized that this check is not entirely precise. When
>doing the memory-to-shadow mapping, the memory address always has its
>top byte set to 0xff: both the inlined compiler code and the outline
>KASAN code do this

Do you mean that non-canonical addresses passed to kasan_mem_to_shadow() will
map to the same space that the canonical version would map to?

What does that? Does the compiler do something more than is in
kasan_mem_to_shadow() when instrumenting functions?

> 		    Thus, the possible values a shadow address can
>take are the result of the memory-to-shadow mapping applied to
>[0xff00000000000000, 0xffffffffffffffff], not to the whole address
>space. So we can make this check more precise.

In case my question above didn't lead to this: what happens to the rest of the
values if they get plugged into kasan_mem_to_shadow()?

>
>> +       }
>>
>>         orig_addr = (unsigned long)kasan_shadow_to_mem((void *)addr);
>>
>> diff --git a/scripts/gdb/linux/mm.py b/scripts/gdb/linux/mm.py
>> index 7571aebbe650..2e63f3dedd53 100644
>> --- a/scripts/gdb/linux/mm.py
>> +++ b/scripts/gdb/linux/mm.py
>> @@ -110,12 +110,13 @@ class aarch64_page_ops():
>>          self.KERNEL_END = gdb.parse_and_eval("_end")
>>
>>          if constants.LX_CONFIG_KASAN_GENERIC or constants.LX_CONFIG_KASAN_SW_TAGS:
>> +            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
>>              if constants.LX_CONFIG_KASAN_GENERIC:
>>                  self.KASAN_SHADOW_SCALE_SHIFT = 3
>> +                self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
>>              else:
>>                  self.KASAN_SHADOW_SCALE_SHIFT = 4
>> -            self.KASAN_SHADOW_OFFSET = constants.LX_CONFIG_KASAN_SHADOW_OFFSET
>> -            self.KASAN_SHADOW_END = (1 << (64 - self.KASAN_SHADOW_SCALE_SHIFT)) + self.KASAN_SHADOW_OFFSET
>> +                self.KASAN_SHADOW_END = self.KASAN_SHADOW_OFFSET
>>              self.PAGE_END = self.KASAN_SHADOW_END - (1 << (self.vabits_actual - self.KASAN_SHADOW_SCALE_SHIFT))
>>          else:
>>              self.PAGE_END = self._PAGE_END(self.VA_BITS_MIN)
>
>We likely also need to update scripts/gdb/linux/kasan.py.
>
>Also, later in the series, you change KASAN_SHADOW_OFFSET from a
>config option into a runtime_const, which AFAIU would make these
>scripts stop working.

Thanks, I'll have to think about these.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 02/14] kasan: sw_tags: Check kasan_flag_enabled at runtime

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:30:09 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:16 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> From: Samuel Holland <samuel.holland@sifive.com>
>>
>> On RISC-V, the ISA extension required to dereference tagged pointers is
>> optional, and the interface to enable pointer masking requires firmware
>> support. Therefore, we must detect at runtime if sw_tags is usable on a
>> given machine. Reuse the logic from hw_tags to dynamically enable KASAN.
>
>Is this patch required on x86 as well? If so, I think it makes sense
>to point it out here. And do the same in messages for other commits
>that now mention RISC-V.

Not really necessary, I just thought all the general kasan patches from the
risc-v series could be added here at once. But you're right, I'll let Samuel
send these two (2nd and 3rd) patches since they relate to risc-v and not x86.

>
>>
>> This commit makes no functional change to the KASAN_HW_TAGS code path.
>>
>> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
>> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>>  include/linux/kasan-enabled.h | 15 +++++----------
>>  mm/kasan/hw_tags.c            | 10 ----------
>>  mm/kasan/tags.c               | 10 ++++++++++
>>  3 files changed, 15 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/kasan-enabled.h b/include/linux/kasan-enabled.h
>> index 6f612d69ea0c..648bda9495b7 100644
>> --- a/include/linux/kasan-enabled.h
>> +++ b/include/linux/kasan-enabled.h
>> @@ -4,7 +4,7 @@
>>
>>  #include <linux/static_key.h>
>>
>> -#ifdef CONFIG_KASAN_HW_TAGS
>> +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS)
>>
>>  DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled);
>>
>> @@ -13,23 +13,18 @@ static __always_inline bool kasan_enabled(void)
>>         return static_branch_likely(&kasan_flag_enabled);
>>  }
>>
>> -static inline bool kasan_hw_tags_enabled(void)
>> -{
>> -       return kasan_enabled();
>> -}
>> -
>> -#else /* CONFIG_KASAN_HW_TAGS */
>> +#else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>>
>>  static inline bool kasan_enabled(void)
>>  {
>>         return IS_ENABLED(CONFIG_KASAN);
>>  }
>>
>> +#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */
>> +
>>  static inline bool kasan_hw_tags_enabled(void)
>>  {
>> -       return false;
>> +       return IS_ENABLED(CONFIG_KASAN_HW_TAGS) && kasan_enabled();
>>  }
>>
>> -#endif /* CONFIG_KASAN_HW_TAGS */
>> -
>>  #endif /* LINUX_KASAN_ENABLED_H */
>> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
>> index 9a6927394b54..7f82af13b6a6 100644
>> --- a/mm/kasan/hw_tags.c
>> +++ b/mm/kasan/hw_tags.c
>> @@ -45,13 +45,6 @@ static enum kasan_arg kasan_arg __ro_after_init;
>>  static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
>>  static enum kasan_arg_vmalloc kasan_arg_vmalloc __initdata;
>>
>> -/*
>> - * Whether KASAN is enabled at all.
>> - * The value remains false until KASAN is initialized by kasan_init_hw_tags().
>> - */
>> -DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
>> -EXPORT_SYMBOL(kasan_flag_enabled);
>> -
>>  /*
>>   * Whether the selected mode is synchronous, asynchronous, or asymmetric.
>>   * Defaults to KASAN_MODE_SYNC.
>> @@ -259,9 +252,6 @@ void __init kasan_init_hw_tags(void)
>>
>>         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",
>>                 kasan_mode_info(),
>>                 str_on_off(kasan_vmalloc_enabled()),
>> diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
>> index d65d48b85f90..c111d98961ed 100644
>> --- a/mm/kasan/tags.c
>> +++ b/mm/kasan/tags.c
>> @@ -32,6 +32,13 @@ enum kasan_arg_stacktrace {
>>
>>  static enum kasan_arg_stacktrace kasan_arg_stacktrace __initdata;
>>
>> +/*
>> + * Whether KASAN is enabled at all.
>> + * The value remains false until KASAN is initialized by kasan_init_tags().
>> + */
>> +DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled);
>> +EXPORT_SYMBOL(kasan_flag_enabled);
>> +
>>  /* Whether to collect alloc/free stack traces. */
>>  DEFINE_STATIC_KEY_TRUE(kasan_flag_stacktrace);
>>
>> @@ -92,6 +99,9 @@ void __init kasan_init_tags(void)
>>                 if (WARN_ON(!stack_ring.entries))
>>                         static_branch_disable(&kasan_flag_stacktrace);
>>         }
>> +
>> +       /* KASAN is now initialized, enable it. */
>> +       static_branch_enable(&kasan_flag_enabled);
>>  }
>>
>>  static void save_stack_info(struct kmem_cache *cache, void *object,
>> --
>> 2.47.1
>>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:31:26 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> Make CONFIG_KASAN_SW_TAGS available for x86 machines if they have
>> ADDRESS_MASKING enabled (LAM) as that works similarly to Top-Byte Ignore
>> (TBI) that allows the software tag-based mode on arm64 platform.
>>
>> Set scale macro based on KASAN mode: in software tag-based mode 32 bytes
>> of memory map to one shadow byte and 16 in generic mode.
>
>These should be 16 and 8.

Thanks, I thought I got all the dense mode stuff from this series.

>
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v2:
>> - Remove KASAN dense code.
>>
>>  arch/x86/Kconfig                | 6 ++++++
>>  arch/x86/boot/compressed/misc.h | 1 +
>>  arch/x86/include/asm/kasan.h    | 2 +-
>>  arch/x86/kernel/setup.c         | 2 ++
>>  4 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index f4ef64bf824a..dc48eb5b664f 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -195,6 +195,7 @@ config X86
>>         select HAVE_ARCH_JUMP_LABEL_RELATIVE
>>         select HAVE_ARCH_KASAN                  if X86_64
>>         select HAVE_ARCH_KASAN_VMALLOC          if X86_64
>> +       select HAVE_ARCH_KASAN_SW_TAGS          if ADDRESS_MASKING
>>         select HAVE_ARCH_KFENCE
>>         select HAVE_ARCH_KMSAN                  if X86_64
>>         select HAVE_ARCH_KGDB
>> @@ -402,6 +403,11 @@ config KASAN_SHADOW_OFFSET
>>         hex
>>         default 0xdffffc0000000000 if KASAN_GENERIC
>>
>> +config KASAN_SHADOW_SCALE_SHIFT
>> +       int
>> +       default 4 if KASAN_SW_TAGS
>> +       default 3
>
>What's the purpose of this config option? I think we can just change
>the value of the KASAN_SHADOW_SCALE_SHIFT define when KASAN_SW_TAGS is
>enabled.

Well, I was aiming at later adding the "default 5 if KASAN_SW_TAGS_DENSE", and
this way it would look much cleaner than the:

if KASAN_SW_TAGS
	if KASAN_SW_TAGS_DENSE
		KASAN_SHADOW_SCALE_SHIFT = 5
	else
		KASAN_SHADOW_SCALE_SHIFT = 4
else
	KASAN_SHADOW_SCALE_SHIFT = 3

But now that I think of it, it should be possible to overwrite the
KASAN_SHADOW_SCALE_SHIFT from non-arch code if dense mode is enabled.

That's a topic for the next series but I'd imagine all architectures would
normally use the 16 memory bytes / shadow byte and if they'd care for the dense
mode they'd go for 32 memory bytes / shadow byte. Or do you think that's a
faulty assumption?

>
>> +
>>  config HAVE_INTEL_TXT
>>         def_bool y
>>         depends on INTEL_IOMMU && ACPI
>> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
>> index dd8d1a85f671..f6a87e9ad200 100644
>> --- a/arch/x86/boot/compressed/misc.h
>> +++ b/arch/x86/boot/compressed/misc.h
>> @@ -13,6 +13,7 @@
>>  #undef CONFIG_PARAVIRT_SPINLOCKS
>>  #undef CONFIG_KASAN
>>  #undef CONFIG_KASAN_GENERIC
>> +#undef CONFIG_KASAN_SW_TAGS
>>
>>  #define __NO_FORTIFY
>>
>> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
>> index 4bfd3641af84..cfc31e4a2f70 100644
>> --- a/arch/x86/include/asm/kasan.h
>> +++ b/arch/x86/include/asm/kasan.h
>> @@ -6,7 +6,7 @@
>>  #include <linux/kasan-tags.h>
>>  #include <linux/types.h>
>>
>> -#define KASAN_SHADOW_SCALE_SHIFT 3
>> +#define KASAN_SHADOW_SCALE_SHIFT CONFIG_KASAN_SHADOW_SCALE_SHIFT
>>
>>  /*
>>   * Compiler uses shadow offset assuming that addresses start
>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index cebee310e200..768990c573ea 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -1124,6 +1124,8 @@ void __init setup_arch(char **cmdline_p)
>>
>>         kasan_init();
>>
>> +       kasan_init_sw_tags();
>> +
>>         /*
>>          * Sync back kernel address range.
>>          *
>> --
>> 2.47.1
>>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-20 at 00:31:08 +0100, Andrey Konovalov wrote:
>On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> On x86, generic KASAN is setup in a way that needs a single
>> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
>> to facilitate boot time switching and it's a compiler ABI so it can't be
>> changed during runtime.
>>
>> Software tag-based mode doesn't tie shadow start and end to any linear
>> addresses as part of the compiler ABI so it can be changed during
>> runtime.
>
>KASAN_SHADOW_OFFSET is passed to the compiler via
>hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
>mode). So while we can change its value, it has to be known at compile
>time. So I don't think using a runtime constant would work.

I don't know about arm64, but this doesn't seem to work right now on x86. I
think I recall that hwasan-mapping-offset isn't implemented on the x86 LLVM or
something like that? I'm sure I saw some note about it a while ago on the
internet but I couldn't find it today.

Anyway if KASAN_SHADOW_OFFSET is not set at compile time it defaults to nothing
and just doesn't get passed into kasan-params a few lines below. I assume that
result seems a little too makeshift for runtime const to make sense here?

>
>Which means that KASAN_SHADOW_OFFSET has to have such a value that
>works for both 4 and 5 level page tables. This possibly means we might
>need something different than the first patch in this series.

I'll think again about doing one offset for both paging levels so that it's as
optimal as possible.

>
>But in case I'm wrong, I left comments for the current code below.
>
>> This notion, for KASAN purposes, allows to optimize out macros
>> such us pgtable_l5_enabled() which would otherwise be used in every
>> single KASAN related function.
>>
>> Use runtime_const infrastructure with pgtable_l5_enabled() to initialize
>> the end address of KASAN's shadow address space. It's a good choice
>> since in software tag based mode KASAN_SHADOW_OFFSET and
>> KASAN_SHADOW_END refer to the same value and the offset in
>> kasan_mem_to_shadow() is a signed negative value.
>>
>> Setup KASAN_SHADOW_END values so that they're aligned to 4TB in 4-level
>> paging mode and to 2PB in 5-level paging mode. Also update x86 memory
>> map documentation.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v2:
>> - Change documentation kasan start address to non-dense values.
>>
>>  Documentation/arch/x86/x86_64/mm.rst |  6 ++++--
>>  arch/x86/Kconfig                     |  3 +--
>>  arch/x86/include/asm/kasan.h         | 14 +++++++++++++-
>>  arch/x86/kernel/vmlinux.lds.S        |  1 +
>>  arch/x86/mm/kasan_init_64.c          |  5 ++++-
>>  5 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/arch/x86/x86_64/mm.rst b/Documentation/arch/x86/x86_64/mm.rst
>> index f2db178b353f..5014ec322e19 100644
>> --- a/Documentation/arch/x86/x86_64/mm.rst
>> +++ b/Documentation/arch/x86/x86_64/mm.rst
>> @@ -60,7 +60,8 @@ Complete virtual memory map with 4-level page tables
>>     ffffe90000000000 |  -23    TB | ffffe9ffffffffff |    1 TB | ... unused hole
>>     ffffea0000000000 |  -22    TB | ffffeaffffffffff |    1 TB | virtual memory map (vmemmap_base)
>>     ffffeb0000000000 |  -21    TB | ffffebffffffffff |    1 TB | ... unused hole
>> -   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory
>> +   ffffec0000000000 |  -20    TB | fffffbffffffffff |   16 TB | KASAN shadow memory (generic mode)
>> +   fffff40000000000 |   -8    TB | fffffc0000000000 |    8 TB | KASAN shadow memory (software tag-based mode)
>>    __________________|____________|__________________|_________|____________________________________________________________
>>                                                                |
>>                                                                | Identical layout to the 56-bit one from here on:
>> @@ -130,7 +131,8 @@ Complete virtual memory map with 5-level page tables
>>     ffd2000000000000 |  -11.5  PB | ffd3ffffffffffff |  0.5 PB | ... unused hole
>>     ffd4000000000000 |  -11    PB | ffd5ffffffffffff |  0.5 PB | virtual memory map (vmemmap_base)
>>     ffd6000000000000 |  -10.5  PB | ffdeffffffffffff | 2.25 PB | ... unused hole
>> -   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory
>> +   ffdf000000000000 |   -8.25 PB | fffffbffffffffff |   ~8 PB | KASAN shadow memory (generic mode)
>> +   ffe0000000000000 |   -6    PB | fff0000000000000 |    4 PB | KASAN shadow memory (software tag-based mode)
>>    __________________|____________|__________________|_________|____________________________________________________________
>>                                                                |
>>                                                                | Identical layout to the 47-bit one from here on:
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 6df7779ed6da..f4ef64bf824a 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -400,8 +400,7 @@ config AUDIT_ARCH
>>
>>  config KASAN_SHADOW_OFFSET
>>         hex
>> -       depends on KASAN
>> -       default 0xdffffc0000000000
>> +       default 0xdffffc0000000000 if KASAN_GENERIC
>
>Let's put a comment here explaining what happens if !KASAN_GENERIC.
>
>Also, as I mentioned in the first patch, we need to figure out what to
>do with scripts/gdb/linux/kasan.py.

I'll look through the scripts. Maybe it's possible to figure out if 5-level
paging is enabled from there and setup the kasan offset based on that.

>
>>
>>  config HAVE_INTEL_TXT
>>         def_bool y
>> diff --git a/arch/x86/include/asm/kasan.h b/arch/x86/include/asm/kasan.h
>> index a75f0748a4b6..4bfd3641af84 100644
>> --- a/arch/x86/include/asm/kasan.h
>> +++ b/arch/x86/include/asm/kasan.h
>> @@ -5,7 +5,7 @@
>>  #include <linux/const.h>
>>  #include <linux/kasan-tags.h>
>>  #include <linux/types.h>
>> -#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>> +
>>  #define KASAN_SHADOW_SCALE_SHIFT 3
>>
>>  /*
>> @@ -14,6 +14,8 @@
>>   * for kernel really starts from compiler's shadow offset +
>>   * 'kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT
>>   */
>> +#ifdef CONFIG_KASAN_GENERIC
>> +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL)
>>  #define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
>>                                         ((-1UL << __VIRTUAL_MASK_SHIFT) >> \
>>                                                 KASAN_SHADOW_SCALE_SHIFT))
>> @@ -24,12 +26,22 @@
>>  #define KASAN_SHADOW_END        (KASAN_SHADOW_START + \
>>                                         (1ULL << (__VIRTUAL_MASK_SHIFT - \
>>                                                   KASAN_SHADOW_SCALE_SHIFT)))
>> +#endif
>> +
>>
>>  #ifndef __ASSEMBLY__
>> +#include <asm/runtime-const.h>
>>  #include <linux/bitops.h>
>>  #include <linux/bitfield.h>
>>  #include <linux/bits.h>
>>
>> +#ifdef CONFIG_KASAN_SW_TAGS
>> +extern unsigned long KASAN_SHADOW_END_RC;
>> +#define KASAN_SHADOW_END       runtime_const_ptr(KASAN_SHADOW_END_RC)
>> +#define KASAN_SHADOW_OFFSET    KASAN_SHADOW_END
>> +#define KASAN_SHADOW_START     (KASAN_SHADOW_END - ((UL(1)) << (__VIRTUAL_MASK_SHIFT - KASAN_SHADOW_SCALE_SHIFT)))
>
>Any reason these are under __ASSEMBLY__? They seem to belong better
>together with the CONFIG_KASAN_GENERIC definitions above.

I remember getting a wall of odd looking compile errors when this wasn't under
assembly. But I'll recheck.

>
>> +#endif
>> +
>>  #define arch_kasan_set_tag(addr, tag)  __tag_set(addr, tag)
>>  #define arch_kasan_reset_tag(addr)     __tag_reset(addr)
>>  #define arch_kasan_get_tag(addr)       __tag_get(addr)
>> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
>> index 0deb4887d6e9..df6c85f8f48f 100644
>> --- a/arch/x86/kernel/vmlinux.lds.S
>> +++ b/arch/x86/kernel/vmlinux.lds.S
>> @@ -353,6 +353,7 @@ SECTIONS
>>
>>         RUNTIME_CONST_VARIABLES
>>         RUNTIME_CONST(ptr, USER_PTR_MAX)
>> +       RUNTIME_CONST(ptr, KASAN_SHADOW_END_RC)
>>
>>         . = ALIGN(PAGE_SIZE);
>>
>> diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
>> index 299a2144dac4..5ca5862a5cd6 100644
>> --- a/arch/x86/mm/kasan_init_64.c
>> +++ b/arch/x86/mm/kasan_init_64.c
>> @@ -358,6 +358,9 @@ void __init kasan_init(void)
>>         int i;
>>
>>         memcpy(early_top_pgt, init_top_pgt, sizeof(early_top_pgt));
>> +       unsigned long KASAN_SHADOW_END_RC = pgtable_l5_enabled() ? 0xfff0000000000000 : 0xfffffc0000000000;
>
>I think defining these constants in arch/x86/include/asm/kasan.h is
>cleaner than hardcoding them here.
>

Okay, I'll change that.

>
>
>
>
>
>
>> +
>> +       runtime_const_init(ptr, KASAN_SHADOW_END_RC);
>>
>>         /*
>>          * We use the same shadow offset for 4- and 5-level paging to
>> @@ -372,7 +375,7 @@ void __init kasan_init(void)
>>          * bunch of things like kernel code, modules, EFI mapping, etc.
>>          * We need to take extra steps to not overwrite them.
>>          */
>> -       if (pgtable_l5_enabled()) {
>> +       if (pgtable_l5_enabled() && !IS_ENABLED(CONFIG_KASAN_SW_TAGS)) {
>>                 void *ptr;
>>
>>                 ptr = (void *)pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_END));
>> --
>> 2.47.1
>>

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-21 at 16:10:54 +0100, Maciej Wieczor-Retman wrote:
>On 2025-02-20 at 00:31:08 +0100, Andrey Konovalov wrote:
>>On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
>><maciej.wieczor-retman@intel.com> wrote:
>>>
>>> On x86, generic KASAN is setup in a way that needs a single
>>> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
>>> to facilitate boot time switching and it's a compiler ABI so it can't be
>>> changed during runtime.
>>>
>>> Software tag-based mode doesn't tie shadow start and end to any linear
>>> addresses as part of the compiler ABI so it can be changed during
>>> runtime.
>>
>>KASAN_SHADOW_OFFSET is passed to the compiler via
>>hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
>>mode). So while we can change its value, it has to be known at compile
>>time. So I don't think using a runtime constant would work.
>
>I don't know about arm64, but this doesn't seem to work right now on x86. I
>think I recall that hwasan-mapping-offset isn't implemented on the x86 LLVM or
>something like that? I'm sure I saw some note about it a while ago on the
>internet but I couldn't find it today.
>
>Anyway if KASAN_SHADOW_OFFSET is not set at compile time it defaults to nothing
>and just doesn't get passed into kasan-params a few lines below. I assume that
>result seems a little too makeshift for runtime const to make sense here?
>
>>
>>Which means that KASAN_SHADOW_OFFSET has to have such a value that
>>works for both 4 and 5 level page tables. This possibly means we might
>>need something different than the first patch in this series.
>
>I'll think again about doing one offset for both paging levels so that it's as
>optimal as possible.
>

Also I was wondering if you know what "hwasan-mapping-offset-dynamic" option is?
I noticed it in the llvm docs but can't find a good description what it does,
even from looking at the code in HWAddressSanitizer.cpp. If
hwasan-mapping-offset is not implemeneted for x86 I doubt this is but maybe it
could help in a cleaner makefile for x86 at least? Especially once these options
will be working in x86 llvm.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >Is there any reason we need this change for x86 SW_TAGS besides the
> >optimization benefits?
>
> I wanted to have the shadow memory boundries aligned properly, to not waste page
> table entries, so the memory map is more straight forward. This patch helps with
> that, I don't think it would have worked without it.

Ok, I see - let's add this info into the commit message then.

> >However, I just realized that this check is not entirely precise. When
> >doing the memory-to-shadow mapping, the memory address always has its
> >top byte set to 0xff: both the inlined compiler code and the outline
> >KASAN code do this
>
> Do you mean that non-canonical addresses passed to kasan_mem_to_shadow() will
> map to the same space that the canonical version would map to?

No, but non-canonical address are never passed to
kasan_mem_to_shadow(): KASAN always resets the tag before calling this
function.

> What does that? Does the compiler do something more than is in
> kasan_mem_to_shadow() when instrumenting functions?

Same for the compiler, it always untags the pointer first [1].

[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L922

> >                   Thus, the possible values a shadow address can
> >take are the result of the memory-to-shadow mapping applied to
> >[0xff00000000000000, 0xffffffffffffffff], not to the whole address
> >space. So we can make this check more precise.
>
> In case my question above didn't lead to this: what happens to the rest of the
> values if they get plugged into kasan_mem_to_shadow()?

We will get some invalid addresses. But this should never happen in
the first place.

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[Andrey Konovalov]

On Fri, Feb 21, 2025 at 3:45 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >What's the purpose of this config option? I think we can just change
> >the value of the KASAN_SHADOW_SCALE_SHIFT define when KASAN_SW_TAGS is
> >enabled.
>
> Well, I was aiming at later adding the "default 5 if KASAN_SW_TAGS_DENSE", and
> this way it would look much cleaner than the:
>
> if KASAN_SW_TAGS
>         if KASAN_SW_TAGS_DENSE
>                 KASAN_SHADOW_SCALE_SHIFT = 5
>         else
>                 KASAN_SHADOW_SCALE_SHIFT = 4
> else
>         KASAN_SHADOW_SCALE_SHIFT = 3

I think this is fine. It's still better than adding a non-configurable
config option.

> But now that I think of it, it should be possible to overwrite the
> KASAN_SHADOW_SCALE_SHIFT from non-arch code if dense mode is enabled.

This should also work. Especially since the dense mode will probably
work for arm64 as well.

But let's keep this series self-contained.

> That's a topic for the next series but I'd imagine all architectures would
> normally use the 16 memory bytes / shadow byte and if they'd care for the dense
> mode they'd go for 32 memory bytes / shadow byte. Or do you think that's a
> faulty assumption?

Probably, but for sure I don't know, not that many architectures that
care about memory tagging yet :)

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Fri, Feb 21, 2025 at 4:11 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> On 2025-02-20 at 00:31:08 +0100, Andrey Konovalov wrote:
> >On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
> ><maciej.wieczor-retman@intel.com> wrote:
> >>
> >> On x86, generic KASAN is setup in a way that needs a single
> >> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
> >> to facilitate boot time switching and it's a compiler ABI so it can't be
> >> changed during runtime.
> >>
> >> Software tag-based mode doesn't tie shadow start and end to any linear
> >> addresses as part of the compiler ABI so it can be changed during
> >> runtime.
> >
> >KASAN_SHADOW_OFFSET is passed to the compiler via
> >hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
> >mode). So while we can change its value, it has to be known at compile
> >time. So I don't think using a runtime constant would work.
>
> I don't know about arm64, but this doesn't seem to work right now on x86.

You mean it _does_ seem to work? Or otherwise if runtime constant
doesn't work on x86, then we shouldn't use it?

> I
> think I recall that hwasan-mapping-offset isn't implemented on the x86 LLVM or
> something like that? I'm sure I saw some note about it a while ago on the
> internet but I couldn't find it today.

In LLVM sources, ShadowBase gets calculated [1] based on
Mapping.Offset [2], which is in turn taken [3] from
hwasan-mapping-offset [4]. And then ShadowBase is used to calculate
[5] the shadow memory address.

All of this happens in the common code, so this should affect both x86
and arm64.

[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1305
[2] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L761
[3] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1863
[4] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L171
[5] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L899

>
> Anyway if KASAN_SHADOW_OFFSET is not set at compile time it defaults to nothing
> and just doesn't get passed into kasan-params a few lines below. I assume that
> result seems a little too makeshift for runtime const to make sense here?

Sorry, I don't understand this question.

If hwasan-mapping-offset is not set properly, then in the inline
instrumentation mode, the compiler won't generate the right
instructions to calculate the shadow memory address.

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Fri, Feb 21, 2025 at 4:27 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> Also I was wondering if you know what "hwasan-mapping-offset-dynamic" option is?
> I noticed it in the llvm docs but can't find a good description what it does,
> even from looking at the code in HWAddressSanitizer.cpp. If
> hwasan-mapping-offset is not implemeneted for x86 I doubt this is but maybe it
> could help in a cleaner makefile for x86 at least? Especially once these options
> will be working in x86 llvm.

Yeah, reading the code [1] works better to understand these options.

My understanding is that specifying
-hwasan-mapping-offset-dynamic=global without -hwasan-mapping-offset
would make the generated code get the shadow memory address from the
__hwasan_shadow_memory_dynamic_address global variable. This option is
in the common code, so it should work on x86 too. But I believe it's
intended for userspace, not sure if it would work for the kernel.

Even if we use this option, I don't get how it would make the Makefile
cleaner - to me it already looks clean tbh :) But it will possibly
make KASAN work slower.

[1] https://github.com/llvm/llvm-project/blob/llvmorg-21-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1929

Re: [PATCH v2 14/14] x86: Make software tag-based kasan available

[Maciej Wieczor-Retman]

On 2025-02-22 at 16:06:34 +0100, Andrey Konovalov wrote:
>On Fri, Feb 21, 2025 at 3:45 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >What's the purpose of this config option? I think we can just change
>> >the value of the KASAN_SHADOW_SCALE_SHIFT define when KASAN_SW_TAGS is
>> >enabled.
>>
>> Well, I was aiming at later adding the "default 5 if KASAN_SW_TAGS_DENSE", and
>> this way it would look much cleaner than the:
>>
>> if KASAN_SW_TAGS
>>         if KASAN_SW_TAGS_DENSE
>>                 KASAN_SHADOW_SCALE_SHIFT = 5
>>         else
>>                 KASAN_SHADOW_SCALE_SHIFT = 4
>> else
>>         KASAN_SHADOW_SCALE_SHIFT = 3
>
>I think this is fine. It's still better than adding a non-configurable
>config option.
>
>> But now that I think of it, it should be possible to overwrite the
>> KASAN_SHADOW_SCALE_SHIFT from non-arch code if dense mode is enabled.
>
>This should also work. Especially since the dense mode will probably
>work for arm64 as well.
>
>But let's keep this series self-contained.

Yes, of course. Anyway I'll just do one preprocessor if else in the same place
that the old x86 KASAN_SHADOW_SCALE_SHIFT was.

>
>> That's a topic for the next series but I'd imagine all architectures would
>> normally use the 16 memory bytes / shadow byte and if they'd care for the dense
>> mode they'd go for 32 memory bytes / shadow byte. Or do you think that's a
>> faulty assumption?
>
>Probably, but for sure I don't know, not that many architectures that
>care about memory tagging yet :)

I'll keep this assumption for now then. If some arch will have a different idea
about granularity I suppose the relevant code can be moved to arch specific
directories.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-22 at 16:07:20 +0100, Andrey Konovalov wrote:
>On Fri, Feb 21, 2025 at 4:11 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> On 2025-02-20 at 00:31:08 +0100, Andrey Konovalov wrote:
>> >On Tue, Feb 18, 2025 at 9:20 AM Maciej Wieczor-Retman
>> ><maciej.wieczor-retman@intel.com> wrote:
>> >>
>> >> On x86, generic KASAN is setup in a way that needs a single
>> >> KASAN_SHADOW_OFFSET value for both 4 and 5 level paging. It's required
>> >> to facilitate boot time switching and it's a compiler ABI so it can't be
>> >> changed during runtime.
>> >>
>> >> Software tag-based mode doesn't tie shadow start and end to any linear
>> >> addresses as part of the compiler ABI so it can be changed during
>> >> runtime.
>> >
>> >KASAN_SHADOW_OFFSET is passed to the compiler via
>> >hwasan-mapping-offset, see scripts/Makefile.kasan (for the INLINE
>> >mode). So while we can change its value, it has to be known at compile
>> >time. So I don't think using a runtime constant would work.
>>
>> I don't know about arm64, but this doesn't seem to work right now on x86.
>
>You mean it _does_ seem to work? Or otherwise if runtime constant
>doesn't work on x86, then we shouldn't use it?

I mean in my tests, with setting offset in runtime, everything works correctly
in inline mode. Even though hwasan-mapping-offset ends up empty and doesn't end
up in CFLAGS_KASAN. I assume this means that the inline mode is pretty much the
same as outline mode with the runtime offset setting?

I also tested if hwasan-mapping-offset does anything if I passed random values
to it by hardcoding them in the makefile and still everything seemed to work
just fine. Therefore I assumed that this option doesn't have any effect on x86.

>
>> I
>> think I recall that hwasan-mapping-offset isn't implemented on the x86 LLVM or
>> something like that? I'm sure I saw some note about it a while ago on the
>> internet but I couldn't find it today.
>
>In LLVM sources, ShadowBase gets calculated [1] based on
>Mapping.Offset [2], which is in turn taken [3] from
>hwasan-mapping-offset [4]. And then ShadowBase is used to calculate
>[5] the shadow memory address.
>
>All of this happens in the common code, so this should affect both x86
>and arm64.
>
>[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1305
>[2] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L761
>[3] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1863
>[4] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L171
>[5] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L899

Hmm indeed it does. Then I'm not sure why I didn't crash when I started putting
in random variables. I'll dive into assembly and see what's up in there.

>
>>
>> Anyway if KASAN_SHADOW_OFFSET is not set at compile time it defaults to nothing
>> and just doesn't get passed into kasan-params a few lines below. I assume that
>> result seems a little too makeshift for runtime const to make sense here?
>
>Sorry, I don't understand this question.
>
>If hwasan-mapping-offset is not set properly, then in the inline
>instrumentation mode, the compiler won't generate the right
>instructions to calculate the shadow memory address.

I meant that if we end up with "hwasan-mapping-offset=" because
$(KASAN_SHADOW_OFFSET) didn't return anything, then
$(call check-args, cc-param, $(kasan_params)) drops hwasan-mapping-offset and
doesn't pass it to CFLAGS_KASAN. So I guess then it isn't an issue for the
compiler and inline mode is essentially outline mode.

But anyway I have an idea how to setup the x86 offset for tag-based mode so it
works for both paging modes. I did some testing and value
	0xffeffc0000000000
seems to work fine and has at least some of the benefits I was hoping for when
doing the runtime_const thing. It works in both paging modes because in 5 levels
it's just a little bit below the 0xffe0000000000000 that I was thinking about
first and in 4 levels, because of LAM, it becomes 0xfffffc0000000000 (because in
4 level paging bits 62:48 are masked from address translation. So it's the same
as the end of generic mode shadow memory space.

The alignment doesn't fit the shadow memory size so it's not optimal but I'm not
sure it can be if we want to have the inline mode and python scripts working at
the same time. At the very least I think the KASAN_SHADOW_END won't collide with
other things in the tab-based mode in 5 level paging mode, so no extra steps are
needed (arch/x86/mm/kasan_init_64.c in kasan_init()).

Do you see any problems with this offset for x86 tag-based mode?


Btw I think kasan_check_range() can be optimized on x86 if we use
addr_has_metadata() that doesn't use KASAN_SHADOW_START. Getting rid of it from
the implementation will remove pgtable_l5_enabled() which is pretty slow so
kasan_check_range() which is called a lot would probably work much faster.
Do you see any way in which addr_has_metadata() will make sense but won't use
KASAN_SHADOW_START? Every one of my ideas ends up using pgtable_l5_enabled()
because the metadata can have 6 or 15 bits depending on paging level.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-22 at 16:06:02 +0100, Andrey Konovalov wrote:
>On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >Is there any reason we need this change for x86 SW_TAGS besides the
>> >optimization benefits?
>>
>> I wanted to have the shadow memory boundries aligned properly, to not waste page
>> table entries, so the memory map is more straight forward. This patch helps with
>> that, I don't think it would have worked without it.
>
>Ok, I see - let's add this info into the commit message then.

Sure, but if you like the 0xffeffc0000000000 offset I'll just drop this part.

>
>> >However, I just realized that this check is not entirely precise. When
>> >doing the memory-to-shadow mapping, the memory address always has its
>> >top byte set to 0xff: both the inlined compiler code and the outline
>> >KASAN code do this
>>
>> Do you mean that non-canonical addresses passed to kasan_mem_to_shadow() will
>> map to the same space that the canonical version would map to?
>
>No, but non-canonical address are never passed to
>kasan_mem_to_shadow(): KASAN always resets the tag before calling this
>function.
>
>> What does that? Does the compiler do something more than is in
>> kasan_mem_to_shadow() when instrumenting functions?
>
>Same for the compiler, it always untags the pointer first [1].
>
>[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L922
>
>> >                   Thus, the possible values a shadow address can
>> >take are the result of the memory-to-shadow mapping applied to
>> >[0xff00000000000000, 0xffffffffffffffff], not to the whole address
>> >space. So we can make this check more precise.
>>
>> In case my question above didn't lead to this: what happens to the rest of the
>> values if they get plugged into kasan_mem_to_shadow()?
>
>We will get some invalid addresses. But this should never happen in
>the first place.

Thanks for letting me know about the tag resets, that should make changing the
check in kasan_non_canonical_hook() easier.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-25 at 18:20:08 +0100, Maciej Wieczor-Retman wrote:
>On 2025-02-22 at 16:06:02 +0100, Andrey Konovalov wrote:
>>On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman
>><maciej.wieczor-retman@intel.com> wrote:
>>>
>>> >Is there any reason we need this change for x86 SW_TAGS besides the
>>> >optimization benefits?
>>>
>>> I wanted to have the shadow memory boundries aligned properly, to not waste page
>>> table entries, so the memory map is more straight forward. This patch helps with
>>> that, I don't think it would have worked without it.
>>
>>Ok, I see - let's add this info into the commit message then.
>
>Sure, but if you like the 0xffeffc0000000000 offset I'll just drop this part.
>
>>
>>> >However, I just realized that this check is not entirely precise. When
>>> >doing the memory-to-shadow mapping, the memory address always has its
>>> >top byte set to 0xff: both the inlined compiler code and the outline
>>> >KASAN code do this
>>>
>>> Do you mean that non-canonical addresses passed to kasan_mem_to_shadow() will
>>> map to the same space that the canonical version would map to?
>>
>>No, but non-canonical address are never passed to
>>kasan_mem_to_shadow(): KASAN always resets the tag before calling this
>>function.
>>
>>> What does that? Does the compiler do something more than is in
>>> kasan_mem_to_shadow() when instrumenting functions?
>>
>>Same for the compiler, it always untags the pointer first [1].
>>
>>[1] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L922
>>
>>> >                   Thus, the possible values a shadow address can
>>> >take are the result of the memory-to-shadow mapping applied to
>>> >[0xff00000000000000, 0xffffffffffffffff], not to the whole address
>>> >space. So we can make this check more precise.
>>>
>>> In case my question above didn't lead to this: what happens to the rest of the
>>> values if they get plugged into kasan_mem_to_shadow()?
>>
>>We will get some invalid addresses. But this should never happen in
>>the first place.
>
>Thanks for letting me know about the tag resets, that should make changing the
>check in kasan_non_canonical_hook() easier.

Ah, but the [0xff00000000000000, 0xffffffffffffffff] won't be true for x86
right? Here the tag reset function only resets bits 60:57. So I presume
[0x3e00000000000000, 0xffffffffffffffff] would be the range?

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-25 at 20:12:40 +0100, Maciej Wieczor-Retman wrote:
>On 2025-02-25 at 18:20:08 +0100, Maciej Wieczor-Retman wrote:
>>On 2025-02-22 at 16:06:02 +0100, Andrey Konovalov wrote:
>>>On Fri, Feb 21, 2025 at 2:12 PM Maciej Wieczor-Retman
>>><maciej.wieczor-retman@intel.com> wrote:
>>>> >                   Thus, the possible values a shadow address can
>>>> >take are the result of the memory-to-shadow mapping applied to
>>>> >[0xff00000000000000, 0xffffffffffffffff], not to the whole address
>>>> >space. So we can make this check more precise.
>>>>
>>>> In case my question above didn't lead to this: what happens to the rest of the
>>>> values if they get plugged into kasan_mem_to_shadow()?
>>>
>>>We will get some invalid addresses. But this should never happen in
>>>the first place.
>>
>>Thanks for letting me know about the tag resets, that should make changing the
>>check in kasan_non_canonical_hook() easier.
>
>Ah, but the [0xff00000000000000, 0xffffffffffffffff] won't be true for x86
>right? Here the tag reset function only resets bits 60:57. So I presume
>[0x3e00000000000000, 0xffffffffffffffff] would be the range?

Sorry, brain freeze, I meant [0x1e00000000000000, 0xffffffffffffffff]

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Tue, Feb 25, 2025 at 6:16 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> I mean in my tests, with setting offset in runtime, everything works correctly
> in inline mode. Even though hwasan-mapping-offset ends up empty and doesn't end
> up in CFLAGS_KASAN. I assume this means that the inline mode is pretty much the
> same as outline mode with the runtime offset setting?
>
> I also tested if hwasan-mapping-offset does anything if I passed random values
> to it by hardcoding them in the makefile and still everything seemed to work
> just fine. Therefore I assumed that this option doesn't have any effect on x86.

Hm that's weird. I wonder if inline instrumentation somehow gets auto-disabled.

> Hmm indeed it does. Then I'm not sure why I didn't crash when I started putting
> in random variables. I'll dive into assembly and see what's up in there.

Please do, I'm curious what's going on there.

> But anyway I have an idea how to setup the x86 offset for tag-based mode so it
> works for both paging modes. I did some testing and value
>         0xffeffc0000000000
> seems to work fine and has at least some of the benefits I was hoping for when
> doing the runtime_const thing. It works in both paging modes because in 5 levels
> it's just a little bit below the 0xffe0000000000000 that I was thinking about
> first and in 4 levels, because of LAM, it becomes 0xfffffc0000000000 (because in
> 4 level paging bits 62:48 are masked from address translation. So it's the same
> as the end of generic mode shadow memory space.
>
> The alignment doesn't fit the shadow memory size so it's not optimal but I'm not
> sure it can be if we want to have the inline mode and python scripts working at
> the same time. At the very least I think the KASAN_SHADOW_END won't collide with
> other things in the tab-based mode in 5 level paging mode, so no extra steps are
> needed (arch/x86/mm/kasan_init_64.c in kasan_init()).

What do you mean by "The alignment doesn't fit the shadow memory size"?

> Do you see any problems with this offset for x86 tag-based mode?

I don't, but I think someone who understands the x86 memory layout
better needs to look at this.

> Btw I think kasan_check_range() can be optimized on x86 if we use
> addr_has_metadata() that doesn't use KASAN_SHADOW_START. Getting rid of it from
> the implementation will remove pgtable_l5_enabled() which is pretty slow so
> kasan_check_range() which is called a lot would probably work much faster.
> Do you see any way in which addr_has_metadata() will make sense but won't use
> KASAN_SHADOW_START? Every one of my ideas ends up using pgtable_l5_enabled()
> because the metadata can have 6 or 15 bits depending on paging level.

What if we turn pgtable_l5_enabled() into using a read-only static key
(DEFINE_STATIC_KEY_FALSE_RO) instead of a bool variable? Or if that is
not acceptable, we could cache its value in a KASAN-specific static
key.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Tue, Feb 25, 2025 at 6:21 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >> I wanted to have the shadow memory boundries aligned properly, to not waste page
> >> table entries, so the memory map is more straight forward. This patch helps with
> >> that, I don't think it would have worked without it.
> >
> >Ok, I see - let's add this info into the commit message then.
>
> Sure, but if you like the 0xffeffc0000000000 offset I'll just drop this part.

Sure, assuming it works, I like this address :) But to be fair, I like
any fixed address better than using a runtime const, just to avoid the
complexity.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Tue, Feb 25, 2025 at 9:13 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >>Thanks for letting me know about the tag resets, that should make changing the
> >>check in kasan_non_canonical_hook() easier.
> >
> >Ah, but the [0xff00000000000000, 0xffffffffffffffff] won't be true for x86
> >right? Here the tag reset function only resets bits 60:57. So I presume
> >[0x3e00000000000000, 0xffffffffffffffff] would be the range?
>
> Sorry, brain freeze, I meant [0x1e00000000000000, 0xffffffffffffffff]

+Vitaly, who implemented [1]

Ah, so when the compiler calculates the shadow memory address on x86,
it does | 0x7E (== 0x3F << 1) [2] for when CompileKernel == true,
because LAM uses bits [62:57], I see.

What value can bit 63 and take for _valid kernel_ pointers (on which
KASAN is intended to operate)? If it is always 1, we could arguably
change the compiler to do | 0xFE for CompileKernel. Which would leave
us with only one region to check: [0xfe00000000000000,
0xffffffffffffffff]. But I don't know whether changing the compiler
makes sense: it technically does as instructed by the LAM spec.
(Vitaly, any thoughts? For context: we are discussing how to check
whether a pointer can be a result of a memory-to-shadow mapping
applied to a potentially invalid pointer in kernel HWASAN.)

With the way the compiler works right now, for the perfectly precise
check, I think we need to check 2 ranges: [0xfe00000000000000,
0xffffffffffffffff] for when bit 63 is set (of a potentially-invalid
pointer to which memory-to-shadow mapping is to be applied) and
[0x7e00000000000000, 0x7fffffffffffffff] for when bit 63 is reset. Bit
56 ranges through [0, 1] in both cases.

However, in these patches, you use only bits [60:57]. The compiler is
not aware of this, so it still sets bits [62:57], and we end up with
the same two ranges. But in the KASAN code, you only set bits [60:57],
and thus we can end up with 8 potential ranges (2 possible values for
each of the top 3 bits), which gets complicated. So checking only one
range that covers all of them seems to be reasonable for simplicity
even though not entirely precise. And yes, [0x1e00000000000000,
0xffffffffffffffff] looks like the what we need.

[1] https://github.com/llvm/llvm-project/commit/cb6099ba43b9262a317083858a29fd31af7efa5c
[2] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1259

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-25 at 22:37:37 +0100, Andrey Konovalov wrote:
>On Tue, Feb 25, 2025 at 6:16 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> I mean in my tests, with setting offset in runtime, everything works correctly
>> in inline mode. Even though hwasan-mapping-offset ends up empty and doesn't end
>> up in CFLAGS_KASAN. I assume this means that the inline mode is pretty much the
>> same as outline mode with the runtime offset setting?
>>
>> I also tested if hwasan-mapping-offset does anything if I passed random values
>> to it by hardcoding them in the makefile and still everything seemed to work
>> just fine. Therefore I assumed that this option doesn't have any effect on x86.
>
>Hm that's weird. I wonder if inline instrumentation somehow gets auto-disabled.
>
>> Hmm indeed it does. Then I'm not sure why I didn't crash when I started putting
>> in random variables. I'll dive into assembly and see what's up in there.
>
>Please do, I'm curious what's going on there.

I think I figured it out.

After adding
	kasan_params += hwasan-instrument-with-calls=0
to Makefile.kasan just under
	kasan_params += hwasan-mapping-offset=$(KASAN_SHADOW_OFFSET)
inline works properly in x86. I looked into assembly and before there were just
calls to __hwasan_load/store. After adding the the
hwasan-instrument-with-calls=0 I can see no calls and the KASAN offset is now
inlined, plus all functions that were previously instrumented now have the
kasan_check_range inlined in them.

My LLVM investigation lead me to
	bool shouldInstrumentWithCalls(const Triple &TargetTriple) {
	  return optOr(ClInstrumentWithCalls, TargetTriple.getArch() == Triple::x86_64);
	}
which I assume defaults to "1" on x86? So even with inline mode it doesn't care
and still does an outline version.

I checked how arm64 reacts to adding the hwasan-instrument-with-calls=0 by cross
compiling and I don't see any differences in output assembly.

>
>> But anyway I have an idea how to setup the x86 offset for tag-based mode so it
>> works for both paging modes. I did some testing and value
>>         0xffeffc0000000000
>> seems to work fine and has at least some of the benefits I was hoping for when
>> doing the runtime_const thing. It works in both paging modes because in 5 levels
>> it's just a little bit below the 0xffe0000000000000 that I was thinking about
>> first and in 4 levels, because of LAM, it becomes 0xfffffc0000000000 (because in
>> 4 level paging bits 62:48 are masked from address translation. So it's the same
>> as the end of generic mode shadow memory space.
>>
>> The alignment doesn't fit the shadow memory size so it's not optimal but I'm not
>> sure it can be if we want to have the inline mode and python scripts working at
>> the same time. At the very least I think the KASAN_SHADOW_END won't collide with
>> other things in the tab-based mode in 5 level paging mode, so no extra steps are
>> needed (arch/x86/mm/kasan_init_64.c in kasan_init()).
>
>What do you mean by "The alignment doesn't fit the shadow memory size"?

Maybe that's the wrong way to put it. I meant that KASAN_SHADOW_END and
KASAN_SHADOW_END aren't aligned to the size of shadow memory.

>
>> Do you see any problems with this offset for x86 tag-based mode?
>
>I don't, but I think someone who understands the x86 memory layout
>better needs to look at this.
>
>> Btw I think kasan_check_range() can be optimized on x86 if we use
>> addr_has_metadata() that doesn't use KASAN_SHADOW_START. Getting rid of it from
>> the implementation will remove pgtable_l5_enabled() which is pretty slow so
>> kasan_check_range() which is called a lot would probably work much faster.
>> Do you see any way in which addr_has_metadata() will make sense but won't use
>> KASAN_SHADOW_START? Every one of my ideas ends up using pgtable_l5_enabled()
>> because the metadata can have 6 or 15 bits depending on paging level.
>
>What if we turn pgtable_l5_enabled() into using a read-only static key
>(DEFINE_STATIC_KEY_FALSE_RO) instead of a bool variable? Or if that is
>not acceptable, we could cache its value in a KASAN-specific static
>key.

I think this was a false alarm, sorry. I asked Kirill about turning
pgtable_l5_enabled() into a runtime_const value but it turns out it's already
patched by alternative code during boot. I just saw a bunch more stuff there
because I was looking at the assembly output and the code isn't patched there
yet.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Wed, Feb 26, 2025 at 12:53 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> After adding
>         kasan_params += hwasan-instrument-with-calls=0
> to Makefile.kasan just under
>         kasan_params += hwasan-mapping-offset=$(KASAN_SHADOW_OFFSET)
> inline works properly in x86. I looked into assembly and before there were just
> calls to __hwasan_load/store. After adding the the
> hwasan-instrument-with-calls=0 I can see no calls and the KASAN offset is now
> inlined, plus all functions that were previously instrumented now have the
> kasan_check_range inlined in them.
>
> My LLVM investigation lead me to
>         bool shouldInstrumentWithCalls(const Triple &TargetTriple) {
>           return optOr(ClInstrumentWithCalls, TargetTriple.getArch() == Triple::x86_64);
>         }
> which I assume defaults to "1" on x86? So even with inline mode it doesn't care
> and still does an outline version.

Ah, indeed. Weird discrepancy between x86 and arm.

Florian, Vitaly, do you recall why this was implemented like this?

To account for this, let's then set hwasan-instrument-with-calls=0
when CONFIG_KASAN_INLINE is enabled. And also please add a comment
explaining why this is done.

[...]

> >What do you mean by "The alignment doesn't fit the shadow memory size"?
>
> Maybe that's the wrong way to put it. I meant that KASAN_SHADOW_END and
> KASAN_SHADOW_END aren't aligned to the size of shadow memory.

I see. And the negative side-effect of this would be that we'll need
extra page table entries to describe the shadow region?

[...]

> I think this was a false alarm, sorry. I asked Kirill about turning
> pgtable_l5_enabled() into a runtime_const value but it turns out it's already
> patched by alternative code during boot. I just saw a bunch more stuff there
> because I was looking at the assembly output and the code isn't patched there
> yet.

Great!

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-25 at 22:38:06 +0100, Andrey Konovalov wrote:
>On Tue, Feb 25, 2025 at 9:13 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >>Thanks for letting me know about the tag resets, that should make changing the
>> >>check in kasan_non_canonical_hook() easier.
>> >
>> >Ah, but the [0xff00000000000000, 0xffffffffffffffff] won't be true for x86
>> >right? Here the tag reset function only resets bits 60:57. So I presume
>> >[0x3e00000000000000, 0xffffffffffffffff] would be the range?
>>
>> Sorry, brain freeze, I meant [0x1e00000000000000, 0xffffffffffffffff]
>
>+Vitaly, who implemented [1]
>
>Ah, so when the compiler calculates the shadow memory address on x86,
>it does | 0x7E (== 0x3F << 1) [2] for when CompileKernel == true,
>because LAM uses bits [62:57], I see.

Thanks for the links, now I see what you meant.

>
>What value can bit 63 and take for _valid kernel_ pointers (on which
>KASAN is intended to operate)? If it is always 1, we could arguably
>change the compiler to do | 0xFE for CompileKernel. Which would leave
>us with only one region to check: [0xfe00000000000000,
>0xffffffffffffffff]. But I don't know whether changing the compiler
>makes sense: it technically does as instructed by the LAM spec.
>(Vitaly, any thoughts? For context: we are discussing how to check
>whether a pointer can be a result of a memory-to-shadow mapping
>applied to a potentially invalid pointer in kernel HWASAN.)

With LAM, valid pointers need to have bits 63 and 56 equal for 5 level paging
and bits 63 and 47 equal for 4 level paging. Both set for kernel addresses and
both clear for user addresses.

>With the way the compiler works right now, for the perfectly precise
>check, I think we need to check 2 ranges: [0xfe00000000000000,
>0xffffffffffffffff] for when bit 63 is set (of a potentially-invalid
>pointer to which memory-to-shadow mapping is to be applied) and
>[0x7e00000000000000, 0x7fffffffffffffff] for when bit 63 is reset. Bit
>56 ranges through [0, 1] in both cases.
>
>However, in these patches, you use only bits [60:57]. The compiler is
>not aware of this, so it still sets bits [62:57], and we end up with
>the same two ranges. But in the KASAN code, you only set bits [60:57],
>and thus we can end up with 8 potential ranges (2 possible values for
>each of the top 3 bits), which gets complicated. So checking only one
>range that covers all of them seems to be reasonable for simplicity
>even though not entirely precise. And yes, [0x1e00000000000000,
>0xffffffffffffffff] looks like the what we need.

Aren't the 2 ranges you mentioned in the previous paragraph still valid, no
matter what bits the __tag_set() function uses? I mean bits 62:57 are still
reset by the compiler so bits 62:61 still won't matter. For example addresses
0x1e00000000000000 and 0x3e00000000000000 will resolve to the same thing after
the compiler is done with them right?

>
>[1] https://github.com/llvm/llvm-project/commit/cb6099ba43b9262a317083858a29fd31af7efa5c
>[2] https://github.com/llvm/llvm-project/blob/llvmorg-20-init/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L1259

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-26 at 16:24:28 +0100, Andrey Konovalov wrote:
>On Wed, Feb 26, 2025 at 12:53 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> After adding
>>         kasan_params += hwasan-instrument-with-calls=0
>> to Makefile.kasan just under
>>         kasan_params += hwasan-mapping-offset=$(KASAN_SHADOW_OFFSET)
>> inline works properly in x86. I looked into assembly and before there were just
>> calls to __hwasan_load/store. After adding the the
>> hwasan-instrument-with-calls=0 I can see no calls and the KASAN offset is now
>> inlined, plus all functions that were previously instrumented now have the
>> kasan_check_range inlined in them.
>>
>> My LLVM investigation lead me to
>>         bool shouldInstrumentWithCalls(const Triple &TargetTriple) {
>>           return optOr(ClInstrumentWithCalls, TargetTriple.getArch() == Triple::x86_64);
>>         }
>> which I assume defaults to "1" on x86? So even with inline mode it doesn't care
>> and still does an outline version.
>
>Ah, indeed. Weird discrepancy between x86 and arm.
>
>Florian, Vitaly, do you recall why this was implemented like this?
>
>To account for this, let's then set hwasan-instrument-with-calls=0
>when CONFIG_KASAN_INLINE is enabled. And also please add a comment
>explaining why this is done.

Sure, will do :)

>
>[...]
>
>> >What do you mean by "The alignment doesn't fit the shadow memory size"?
>>
>> Maybe that's the wrong way to put it. I meant that KASAN_SHADOW_END and
>> KASAN_SHADOW_END aren't aligned to the size of shadow memory.
>
>I see. And the negative side-effect of this would be that we'll need
>extra page table entries to describe the shadow region?

I think so, yes. But I guess it's not a big issue, and anyway right now I'm not
sure how to change it so other necessary parts don't break :b

>
>[...]
>
>> I think this was a false alarm, sorry. I asked Kirill about turning
>> pgtable_l5_enabled() into a runtime_const value but it turns out it's already
>> patched by alternative code during boot. I just saw a bunch more stuff there
>> because I was looking at the assembly output and the code isn't patched there
>> yet.
>
>Great!

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Wed, Feb 26, 2025 at 5:43 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >What value can bit 63 and take for _valid kernel_ pointers (on which
> >KASAN is intended to operate)? If it is always 1, we could arguably
> >change the compiler to do | 0xFE for CompileKernel. Which would leave
> >us with only one region to check: [0xfe00000000000000,
> >0xffffffffffffffff]. But I don't know whether changing the compiler
> >makes sense: it technically does as instructed by the LAM spec.
> >(Vitaly, any thoughts? For context: we are discussing how to check
> >whether a pointer can be a result of a memory-to-shadow mapping
> >applied to a potentially invalid pointer in kernel HWASAN.)
>
> With LAM, valid pointers need to have bits 63 and 56 equal for 5 level paging
> and bits 63 and 47 equal for 4 level paging. Both set for kernel addresses and
> both clear for user addresses.

Ah, OK. Then I guess we could even change to compiler to do | 0xFF,
same as arm. But I don't know if this makes sense.

> >With the way the compiler works right now, for the perfectly precise
> >check, I think we need to check 2 ranges: [0xfe00000000000000,
> >0xffffffffffffffff] for when bit 63 is set (of a potentially-invalid
> >pointer to which memory-to-shadow mapping is to be applied) and
> >[0x7e00000000000000, 0x7fffffffffffffff] for when bit 63 is reset. Bit
> >56 ranges through [0, 1] in both cases.
> >
> >However, in these patches, you use only bits [60:57]. The compiler is
> >not aware of this, so it still sets bits [62:57], and we end up with
> >the same two ranges. But in the KASAN code, you only set bits [60:57],
> >and thus we can end up with 8 potential ranges (2 possible values for
> >each of the top 3 bits), which gets complicated. So checking only one
> >range that covers all of them seems to be reasonable for simplicity
> >even though not entirely precise. And yes, [0x1e00000000000000,
> >0xffffffffffffffff] looks like the what we need.
>
> Aren't the 2 ranges you mentioned in the previous paragraph still valid, no
> matter what bits the __tag_set() function uses? I mean bits 62:57 are still
> reset by the compiler so bits 62:61 still won't matter. For example addresses
> 0x1e00000000000000 and 0x3e00000000000000 will resolve to the same thing after
> the compiler is done with them right?

Ah, yes, you're right, it's the same 2 ranges.

I was thinking about the outline instrumentation mode, where the
shadow address would be calculated based on resetting only bits
[60:57]. But then there we have a addr_has_metadata() check in
kasan_check_range(), so KASAN should not try to deference a bad shadow
address and thus should not reach kasan_non_canonical_hook() anyway.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-26 at 20:44:35 +0100, Andrey Konovalov wrote:
>On Wed, Feb 26, 2025 at 5:43 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >What value can bit 63 and take for _valid kernel_ pointers (on which
>> >KASAN is intended to operate)? If it is always 1, we could arguably
>> >change the compiler to do | 0xFE for CompileKernel. Which would leave
>> >us with only one region to check: [0xfe00000000000000,
>> >0xffffffffffffffff]. But I don't know whether changing the compiler
>> >makes sense: it technically does as instructed by the LAM spec.
>> >(Vitaly, any thoughts? For context: we are discussing how to check
>> >whether a pointer can be a result of a memory-to-shadow mapping
>> >applied to a potentially invalid pointer in kernel HWASAN.)
>>
>> With LAM, valid pointers need to have bits 63 and 56 equal for 5 level paging
>> and bits 63 and 47 equal for 4 level paging. Both set for kernel addresses and
>> both clear for user addresses.
>
>Ah, OK. Then I guess we could even change to compiler to do | 0xFF,
>same as arm. But I don't know if this makes sense.

I guess it wouldn't be resetting the tag anymore, just some agreed upon set of
bits. If this argument is just for the non_canonical_hook() purposes I suppose
we can leave it as is and check the two ranges in the kernel.

>
>> >With the way the compiler works right now, for the perfectly precise
>> >check, I think we need to check 2 ranges: [0xfe00000000000000,
>> >0xffffffffffffffff] for when bit 63 is set (of a potentially-invalid
>> >pointer to which memory-to-shadow mapping is to be applied) and
>> >[0x7e00000000000000, 0x7fffffffffffffff] for when bit 63 is reset. Bit
>> >56 ranges through [0, 1] in both cases.
>> >
>> >However, in these patches, you use only bits [60:57]. The compiler is
>> >not aware of this, so it still sets bits [62:57], and we end up with
>> >the same two ranges. But in the KASAN code, you only set bits [60:57],
>> >and thus we can end up with 8 potential ranges (2 possible values for
>> >each of the top 3 bits), which gets complicated. So checking only one
>> >range that covers all of them seems to be reasonable for simplicity
>> >even though not entirely precise. And yes, [0x1e00000000000000,
>> >0xffffffffffffffff] looks like the what we need.
>>
>> Aren't the 2 ranges you mentioned in the previous paragraph still valid, no
>> matter what bits the __tag_set() function uses? I mean bits 62:57 are still
>> reset by the compiler so bits 62:61 still won't matter. For example addresses
>> 0x1e00000000000000 and 0x3e00000000000000 will resolve to the same thing after
>> the compiler is done with them right?
>
>Ah, yes, you're right, it's the same 2 ranges.
>
>I was thinking about the outline instrumentation mode, where the
>shadow address would be calculated based on resetting only bits
>[60:57]. But then there we have a addr_has_metadata() check in
>kasan_check_range(), so KASAN should not try to deference a bad shadow
>address and thus should not reach kasan_non_canonical_hook() anyway.

Okay, so I guess we should do the same check for both arm64 and x86 right? (and
risc-v in the future). Just use the wider range - in this case the 2 ranges that
x86 needs. Then it could look something like:

			// 0xffffffffffffffff maps just below the shadow offset
	if (addr > KASAN_SHADOW_OFFSET ||
			// and check below the most negative address
		(addr < kasan_mem_to_shadow(0xFE << 56) &&
			// biggest positive address that overflows so check both above it
		addr > kasan_mem_to_shadow(~0UL >> 1)) ||
			// smallest positive address but will overflow so check addresses below it
		addr < kasan_mem_to_shadow(0x7E << 56))
		return

so first two lines deal with the first range, and the next two lines deal with
the second one.

Or do you want me to make this part of non_canonical_hook() arch specific for
maximum accuracy?

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-25 at 22:37:58 +0100, Andrey Konovalov wrote:
>On Tue, Feb 25, 2025 at 6:21 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >> I wanted to have the shadow memory boundries aligned properly, to not waste page
>> >> table entries, so the memory map is more straight forward. This patch helps with
>> >> that, I don't think it would have worked without it.
>> >
>> >Ok, I see - let's add this info into the commit message then.
>>
>> Sure, but if you like the 0xffeffc0000000000 offset I'll just drop this part.
>
>Sure, assuming it works, I like this address :) But to be fair, I like
>any fixed address better than using a runtime const, just to avoid the
>complexity.

Btw just out of curiosity on the topic. If we used a runtime specified kasan
offset, could the gdb script issue (not knowing the offset at compile-time) be
fixed by just exporting the value through sysfs?

I know that in inline mode the compiler would still need to know the offset
value but I was curious if this approach was okay at least in outline mode?

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-02-27 at 13:27:32 +0100, Maciej Wieczor-Retman wrote:
>On 2025-02-26 at 20:44:35 +0100, Andrey Konovalov wrote:
>>On Wed, Feb 26, 2025 at 5:43 PM Maciej Wieczor-Retman
>><maciej.wieczor-retman@intel.com> wrote:
>>
>>> >With the way the compiler works right now, for the perfectly precise
>>> >check, I think we need to check 2 ranges: [0xfe00000000000000,
>>> >0xffffffffffffffff] for when bit 63 is set (of a potentially-invalid
>>> >pointer to which memory-to-shadow mapping is to be applied) and
>>> >[0x7e00000000000000, 0x7fffffffffffffff] for when bit 63 is reset. Bit
>>> >56 ranges through [0, 1] in both cases.
>>> >
>>> >However, in these patches, you use only bits [60:57]. The compiler is
>>> >not aware of this, so it still sets bits [62:57], and we end up with
>>> >the same two ranges. But in the KASAN code, you only set bits [60:57],
>>> >and thus we can end up with 8 potential ranges (2 possible values for
>>> >each of the top 3 bits), which gets complicated. So checking only one
>>> >range that covers all of them seems to be reasonable for simplicity
>>> >even though not entirely precise. And yes, [0x1e00000000000000,
>>> >0xffffffffffffffff] looks like the what we need.
>>>
>>> Aren't the 2 ranges you mentioned in the previous paragraph still valid, no
>>> matter what bits the __tag_set() function uses? I mean bits 62:57 are still
>>> reset by the compiler so bits 62:61 still won't matter. For example addresses
>>> 0x1e00000000000000 and 0x3e00000000000000 will resolve to the same thing after
>>> the compiler is done with them right?
>>
>>Ah, yes, you're right, it's the same 2 ranges.
>>
>>I was thinking about the outline instrumentation mode, where the
>>shadow address would be calculated based on resetting only bits
>>[60:57]. But then there we have a addr_has_metadata() check in
>>kasan_check_range(), so KASAN should not try to deference a bad shadow
>>address and thus should not reach kasan_non_canonical_hook() anyway.
>
>Okay, so I guess we should do the same check for both arm64 and x86 right? (and
>risc-v in the future). Just use the wider range - in this case the 2 ranges that
>x86 needs. Then it could look something like:
>
>			// 0xffffffffffffffff maps just below the shadow offset
>	if (addr > KASAN_SHADOW_OFFSET ||
>			// and check below the most negative address
>		(addr < kasan_mem_to_shadow(0xFE << 56) &&
>			// biggest positive address that overflows so check both above it
>		addr > kasan_mem_to_shadow(~0UL >> 1)) ||
>			// smallest positive address but will overflow so check addresses below it
>		addr < kasan_mem_to_shadow(0x7E << 56))
>		return
>
>so first two lines deal with the first range, and the next two lines deal with
>the second one.
>
>Or do you want me to make this part of non_canonical_hook() arch specific for
>maximum accuracy?
>

I was applying your other comments to the series and came up with something like
this. What do you think?

	/*
	 * With the default kasan_mem_to_shadow() algorithm, all addresses
	 * returned by the memory-to-shadow mapping (even for bogus pointers)
	 * must be within a certain displacement from KASAN_SHADOW_OFFSET.
	 *
	 * For Generic KASAN the displacement is unsigned so the mapping from zero
	 * to the last kernel address needs checking.
	 */
	if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
		if (addr < KASAN_SHADOW_OFFSET ||
		    addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
			return;
	} else {
		/*
		 * For the tag-based mode the compiler resets tags in addresses at
		 * the start of kasan_mem_to_shadow(). Because of this it's not
		 * necessary to check a mapping of the entire address space but only
		 * whether a range of [0xFF00000000000000 - 0xFFFFFFFFFFFFFFFF] is a
		 * valid memory-to-shadow mapping. On x86, tags are located in bits
		 * 62:57 so the range becomes [0x7E00000000000000 - 0xFFFFFFFFFFFFFFFF].
		 * The check below tries to exclude invalid addresses by
		 * checking spaces between [0x7E00000000000000 - 0x7FFFFFFFFFFFFFFF]
		 * (which are positive and will overflow the memory-to-shadow
		 * mapping) and [0xFE00000000000000 - 0xFFFFFFFFFFFFFFFF]
		 */
		 if (addr > KASAN_SHADOW_OFFSET ||
		     (addr < (u64)kasan_mem_to_shadow((void *)(0xFEUL << 56)) &&
		     addr > (u64)kasan_mem_to_shadow((void *)(~0UL >> 1))) ||
		     addr < (u64)kasan_mem_to_shadow((void *)(0x7EUL << 56)))
			return;
	}

The comment is a bit long and has a lot of hexes but maybe it's good to leave a
longer explanation so no one has to dig through the mailing archives to
understand the logic :b

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

On Fri, Feb 28, 2025 at 5:13 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> I was applying your other comments to the series and came up with something like
> this. What do you think?
>
>         /*
>          * With the default kasan_mem_to_shadow() algorithm, all addresses
>          * returned by the memory-to-shadow mapping (even for bogus pointers)
>          * must be within a certain displacement from KASAN_SHADOW_OFFSET.
>          *
>          * For Generic KASAN the displacement is unsigned so the mapping from zero
>          * to the last kernel address needs checking.
>          */
>         if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
>                 if (addr < KASAN_SHADOW_OFFSET ||
>                     addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
>                         return;
>         } else {
>                 /*
>                  * For the tag-based mode the compiler resets tags in addresses at
>                  * the start of kasan_mem_to_shadow(). Because of this it's not
>                  * necessary to check a mapping of the entire address space but only
>                  * whether a range of [0xFF00000000000000 - 0xFFFFFFFFFFFFFFFF] is a
>                  * valid memory-to-shadow mapping. On x86, tags are located in bits
>                  * 62:57 so the range becomes [0x7E00000000000000 - 0xFFFFFFFFFFFFFFFF].
>                  * The check below tries to exclude invalid addresses by
>                  * checking spaces between [0x7E00000000000000 - 0x7FFFFFFFFFFFFFFF]
>                  * (which are positive and will overflow the memory-to-shadow
>                  * mapping) and [0xFE00000000000000 - 0xFFFFFFFFFFFFFFFF]
>                  */
>                  if (addr > KASAN_SHADOW_OFFSET ||
>                      (addr < (u64)kasan_mem_to_shadow((void *)(0xFEUL << 56)) &&
>                      addr > (u64)kasan_mem_to_shadow((void *)(~0UL >> 1))) ||
>                      addr < (u64)kasan_mem_to_shadow((void *)(0x7EUL << 56)))
>                         return;
>         }
>
> The comment is a bit long and has a lot of hexes but maybe it's good to leave a
> longer explanation so no one has to dig through the mailing archives to
> understand the logic :b

Explaining the logic sounds good to me!

I think your patch is close to what would look good, but I think the
parentheses in the long if condition look suspicious.

Please check the attached diff (Gmail makes it hard to inline code): I
fixed the parentheses (if I'm right about them being wrong), made the
checks look uniform, added an arm-specific check, and reworked the
comments (please check if they make sense).

If the diff looks good to you, let's use that.

It also would be great, if you could test this: add some code that
dereferences various bad addresses and see if the extra KASAN message
line gets printed during the GPF.

[-- Attachment #2: kasan_non_canonical_hook.patch --]
[-- Type: text/x-patch, Size: 2896 bytes --]

diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 8357e1a33699..7edde1a26a41 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -681,11 +681,56 @@ void kasan_non_canonical_hook(unsigned long addr)
 	const char *bug_type;
 
 	/*
-	 * All addresses that came as a result of the memory-to-shadow mapping
-	 * (even for bogus pointers) must be >= KASAN_SHADOW_OFFSET.
+	 * For Generic KASAN, 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 (addr < KASAN_SHADOW_OFFSET)
-		return;
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
+		if (addr < (u64)kasan_mem_to_shadow((void *)(0UL)) ||
+		    addr > (u64)kasan_mem_to_shadow((void *)(~0UL)))
+			return;
+	}
+
+	/*
+	 * For Software Tag-Based KASAN, kasan_mem_to_shadow() uses the
+	 * arithmetic shift. Normally, this would make checking for a possible
+	 * shadow address complicated, as the shadow address computation
+	 * operation would overflow only for some memory addresses. However, due
+	 * to the chosen KASAN_SHADOW_OFFSET values and the fact the
+	 * kasan_mem_to_shadow() only operates on pointers with the tag reset,
+	 * the overflow always happens (for both x86 and arm64).
+	 *
+	 * For arm64, the top byte of the pointer gets reset to 0xFF. Thus, the
+	 * possible shadow addresses belong to a region that is the result of
+	 * kasan_mem_to_shadow() applied to the memory range
+	 * [0xFF000000000000, 0xFFFFFFFFFFFFFFFF]. Despite the overflow, the
+	 * resulting possible shadow region is contiguous, as the overflow
+	 * happens for both 0xFF000000000000 and 0xFFFFFFFFFFFFFFFF.
+	 */
+	if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) && IS_ENABLED(CONFIG_ARM64)) {
+		if (addr < (u64)kasan_mem_to_shadow((void *)(0xFFUL << 56)) ||
+		    addr > (u64)kasan_mem_to_shadow((void *)(~0UL)))
+			return;
+	}
+
+	 /*
+	  * For x86-64, only the pointer bits [62:57] get reset, and bits #63
+	  * and #56 can be 0 or 1. Thus, kasan_mem_to_shadow() can be possibly
+	  * applied to two regions of memory:
+	  * [0x7E00000000000000, 0x7FFFFFFFFFFFFFFF] and
+	  * [0xFE00000000000000, 0xFFFFFFFFFFFFFFFF]. As the overflow happens
+	  * for both ends of both memory ranges, both possible shadow regions
+	  * are contiguous.
+	 */
+	if (IS_ENABLED(CONFIG_KASAN_SW_TAGS) && IS_ENABLED(CONFIG_X86_64)) {
+		if ((addr < (u64)kasan_mem_to_shadow((void *)(0x7EUL << 56)) ||
+		     addr > (u64)kasan_mem_to_shadow((void *)(~0UL >> 1))) &&
+		    (addr < (u64)kasan_mem_to_shadow((void *)(0xFEUL << 56)) ||
+		     addr > (u64)kasan_mem_to_shadow((void *)(~0UL))))
+			return;
+	}
 
 	orig_addr = (unsigned long)kasan_shadow_to_mem((void *)addr);
 

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Thu, Feb 27, 2025 at 1:33 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> Btw just out of curiosity on the topic. If we used a runtime specified kasan
> offset, could the gdb script issue (not knowing the offset at compile-time) be
> fixed by just exporting the value through sysfs?
>
> I know that in inline mode the compiler would still need to know the offset
> value but I was curious if this approach was okay at least in outline mode?

I think this would work, assuming that GDB can pick it up from sysfs.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-03-01 at 01:22:46 +0100, Andrey Konovalov wrote:
>On Thu, Feb 27, 2025 at 1:33 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> Btw just out of curiosity on the topic. If we used a runtime specified kasan
>> offset, could the gdb script issue (not knowing the offset at compile-time) be
>> fixed by just exporting the value through sysfs?
>>
>> I know that in inline mode the compiler would still need to know the offset
>> value but I was curious if this approach was okay at least in outline mode?
>
>I think this would work, assuming that GDB can pick it up from sysfs.

One other question that came to me about how KASAN works, is there some
mechanism to prevent data races between two threads? In the compiler perhaps?

For example memory is de-allocated and shadow memory is poisoned but some other
thread was just about to do a shadow memory check and was interrupted?

I've read the kasan/vmalloc.c comments and from them I'd extrapolate that the
caller needs to make sure there are not data races / memory barriers are in
place.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-03-01 at 01:21:52 +0100, Andrey Konovalov wrote:
>On Fri, Feb 28, 2025 at 5:13 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> I was applying your other comments to the series and came up with something like
>> this. What do you think?
>>
>>         /*
>>          * With the default kasan_mem_to_shadow() algorithm, all addresses
>>          * returned by the memory-to-shadow mapping (even for bogus pointers)
>>          * must be within a certain displacement from KASAN_SHADOW_OFFSET.
>>          *
>>          * For Generic KASAN the displacement is unsigned so the mapping from zero
>>          * to the last kernel address needs checking.
>>          */
>>         if (IS_ENABLED(CONFIG_KASAN_GENERIC)) {
>>                 if (addr < KASAN_SHADOW_OFFSET ||
>>                     addr >= KASAN_SHADOW_OFFSET + max_shadow_size)
>>                         return;
>>         } else {
>>                 /*
>>                  * For the tag-based mode the compiler resets tags in addresses at
>>                  * the start of kasan_mem_to_shadow(). Because of this it's not
>>                  * necessary to check a mapping of the entire address space but only
>>                  * whether a range of [0xFF00000000000000 - 0xFFFFFFFFFFFFFFFF] is a
>>                  * valid memory-to-shadow mapping. On x86, tags are located in bits
>>                  * 62:57 so the range becomes [0x7E00000000000000 - 0xFFFFFFFFFFFFFFFF].
>>                  * The check below tries to exclude invalid addresses by
>>                  * checking spaces between [0x7E00000000000000 - 0x7FFFFFFFFFFFFFFF]
>>                  * (which are positive and will overflow the memory-to-shadow
>>                  * mapping) and [0xFE00000000000000 - 0xFFFFFFFFFFFFFFFF]
>>                  */
>>                  if (addr > KASAN_SHADOW_OFFSET ||
>>                      (addr < (u64)kasan_mem_to_shadow((void *)(0xFEUL << 56)) &&
>>                      addr > (u64)kasan_mem_to_shadow((void *)(~0UL >> 1))) ||
>>                      addr < (u64)kasan_mem_to_shadow((void *)(0x7EUL << 56)))
>>                         return;
>>         }
>>
>> The comment is a bit long and has a lot of hexes but maybe it's good to leave a
>> longer explanation so no one has to dig through the mailing archives to
>> understand the logic :b
>
>Explaining the logic sounds good to me!
>
>I think your patch is close to what would look good, but I think the
>parentheses in the long if condition look suspicious.
>
>Please check the attached diff (Gmail makes it hard to inline code): I
>fixed the parentheses (if I'm right about them being wrong), made the
>checks look uniform, added an arm-specific check, and reworked the
>comments (please check if they make sense).
>
>If the diff looks good to you, let's use that.
>
>It also would be great, if you could test this: add some code that
>dereferences various bad addresses and see if the extra KASAN message
>line gets printed during the GPF.

Sure, I'll do these tests :)

For now I found my code has some issue with inline mode so I'll first try to
track down what's wrong there.

But looking at the patch you sent I'm wondering - are we treating the arithmetic
in kasan_mem_to_shadow() as unsigned? You wrote that all the ranges will
overflow but I thought we're interpreting the arithmetic as signed - so only
positive addresses will overflow and negative addresses (with bit 63 set) will
only be more negative thus not causing an overflow. That was my assumption when
writing the previous checks - we need to check below the overflown range, above
the negative (not overflown) range, and between the two.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Tue, Mar 4, 2025 at 1:31 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> One other question that came to me about how KASAN works, is there some
> mechanism to prevent data races between two threads? In the compiler perhaps?
>
> For example memory is de-allocated and shadow memory is poisoned but some other
> thread was just about to do a shadow memory check and was interrupted?
>
> I've read the kasan/vmalloc.c comments and from them I'd extrapolate that the
> caller needs to make sure there are not data races / memory barriers are in
> place.

KASAN does nothing to deliberately prevent or detect races. Even if
the race leads to an OOB or UAF, KASAN might not be able to detect it.
But sometimes it does: if poisoned shadow memory values become visible
to the other thread/CPU before it makes a shadow memory value check.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Tue, Mar 4, 2025 at 3:08 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> But looking at the patch you sent I'm wondering - are we treating the arithmetic
> in kasan_mem_to_shadow() as unsigned?

The shift is signed (arithmetic). But for the addition, it doesn't
matter? Adding an integer to a void* pointer should result in the same
value, regardless of whether the integer is signed or unsigned.

> You wrote that all the ranges will
> overflow but I thought we're interpreting the arithmetic as signed - so only
> positive addresses will overflow and negative addresses (with bit 63 set) will
> only be more negative thus not causing an overflow.

Ah, yes, I see what you mean. From the C point of view, calculating
the shadow address for a pointer with bit 63 set can be interpreted as
subtracting from KASAN_SHADOW_OFFSET, and there's no overflow. But on
the assembly level, the compiler should generate the add instruction,
and the addition will overflow in both cases.

The important thing is that both possible shadow memory ranges are
contiguous (either both start and end overflow or none of them).

So this was my brain converting things to assembly. Feel free to
reword/clarify the comments.

> That was my assumption when
> writing the previous checks - we need to check below the overflown range, above
> the negative (not overflown) range, and between the two.

It could be that your checks are equivalent to mine. What I did was to
check that the address lies outside of both contiguous regions, which
makes the checks symmetrical and IMO easier to follow.

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-03-07 at 02:10:39 +0100, Andrey Konovalov wrote:
>On Tue, Mar 4, 2025 at 3:08 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> But looking at the patch you sent I'm wondering - are we treating the arithmetic
>> in kasan_mem_to_shadow() as unsigned?
>
>The shift is signed (arithmetic). But for the addition, it doesn't
>matter? Adding an integer to a void* pointer should result in the same
>value, regardless of whether the integer is signed or unsigned.

Yes, you're right, sorry.

>
>> You wrote that all the ranges will
>> overflow but I thought we're interpreting the arithmetic as signed - so only
>> positive addresses will overflow and negative addresses (with bit 63 set) will
>> only be more negative thus not causing an overflow.
>
>Ah, yes, I see what you mean. From the C point of view, calculating
>the shadow address for a pointer with bit 63 set can be interpreted as
>subtracting from KASAN_SHADOW_OFFSET, and there's no overflow. But on
>the assembly level, the compiler should generate the add instruction,
>and the addition will overflow in both cases.
>
>The important thing is that both possible shadow memory ranges are
>contiguous (either both start and end overflow or none of them).
>
>So this was my brain converting things to assembly. Feel free to
>reword/clarify the comments.

Right, I focused too much on the signed aspect. Treating everything as
overflowing sounds better, more unified.

>
>> That was my assumption when
>> writing the previous checks - we need to check below the overflown range, above
>> the negative (not overflown) range, and between the two.
>
>It could be that your checks are equivalent to mine. What I did was to
>check that the address lies outside of both contiguous regions, which
>makes the checks symmetrical and IMO easier to follow.

I drew this out and yeah, it looks like it's the same, just grouping the logical
expressions differently. What do you think about incorporating something like
the following into your comment about the x86 part? :

	Given the KASAN_SHADOW_OFFSET equal 0xffeffc0000000000
	the following ranges are valid mem-to-shadow mappings:

	0xFFFFFFFFFFFFFFFF
		INVALID
	0xFFEFFBFFFFFFFFFF - kasan_mem_to_shadow(~0UL)
		VALID	- kasan shadow mem
		VALID	- non-canonical kernel virtual address
	0xFFCFFC0000000000 - kasan_mem_to_shadow(0xFEUL << 56)
		INVALID
	0x07EFFBFFFFFFFFFF - kasan_mem_to_shadow(~0UL >> 1)
		VALID	- non-canonical user virtual addresses
		VALID	- user addresses
	0x07CFFC0000000000 - kasan_mem_to_shadow(0x7EUL << 56)
		INVALID
	0x0000000000000000

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Maciej Wieczor-Retman]

On 2025-03-07 at 02:10:12 +0100, Andrey Konovalov wrote:
>On Tue, Mar 4, 2025 at 1:31 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> One other question that came to me about how KASAN works, is there some
>> mechanism to prevent data races between two threads? In the compiler perhaps?
>>
>> For example memory is de-allocated and shadow memory is poisoned but some other
>> thread was just about to do a shadow memory check and was interrupted?
>>
>> I've read the kasan/vmalloc.c comments and from them I'd extrapolate that the
>> caller needs to make sure there are not data races / memory barriers are in
>> place.
>
>KASAN does nothing to deliberately prevent or detect races. Even if
>the race leads to an OOB or UAF, KASAN might not be able to detect it.
>But sometimes it does: if poisoned shadow memory values become visible
>to the other thread/CPU before it makes a shadow memory value check.

Thanks :)

I've came up with a theoretical issue for the following dense series that might
happen if there is some racing but I'll have to experiment if it actually
happens.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Thu, Mar 13, 2025 at 3:58 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >So this was my brain converting things to assembly. Feel free to
> >reword/clarify the comments.
>
> Right, I focused too much on the signed aspect. Treating everything as
> overflowing sounds better, more unified.

Alright!

> >It could be that your checks are equivalent to mine. What I did was to
> >check that the address lies outside of both contiguous regions, which
> >makes the checks symmetrical and IMO easier to follow.
>
> I drew this out and yeah, it looks like it's the same, just grouping the logical
> expressions differently. What do you think about incorporating something like
> the following into your comment about the x86 part? :
>
>         Given the KASAN_SHADOW_OFFSET equal 0xffeffc0000000000
>         the following ranges are valid mem-to-shadow mappings:
>
>         0xFFFFFFFFFFFFFFFF
>                 INVALID
>         0xFFEFFBFFFFFFFFFF - kasan_mem_to_shadow(~0UL)
>                 VALID   - kasan shadow mem
>                 VALID   - non-canonical kernel virtual address
>         0xFFCFFC0000000000 - kasan_mem_to_shadow(0xFEUL << 56)
>                 INVALID
>         0x07EFFBFFFFFFFFFF - kasan_mem_to_shadow(~0UL >> 1)
>                 VALID   - non-canonical user virtual addresses
>                 VALID   - user addresses
>         0x07CFFC0000000000 - kasan_mem_to_shadow(0x7EUL << 56)
>                 INVALID
>         0x0000000000000000

Sounds good - I like this visual representation a lot! Thanks!

Re: [PATCH v2 01/14] kasan: sw_tags: Use arithmetic shift for shadow computation

[Andrey Konovalov]

On Fri, Mar 14, 2025 at 4:58 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >KASAN does nothing to deliberately prevent or detect races. Even if
> >the race leads to an OOB or UAF, KASAN might not be able to detect it.
> >But sometimes it does: if poisoned shadow memory values become visible
> >to the other thread/CPU before it makes a shadow memory value check.
>
> Thanks :)
>
> I've came up with a theoretical issue for the following dense series that might
> happen if there is some racing but I'll have to experiment if it actually
> happens.

As long as it doesn't lead to false positive reports or crashes in the
KASAN runtime - I think it should fine.

Re: [PATCH v2 09/14] mm: Pcpu chunk address tag reset

[Andrey Ryabinin]

On Tue, Feb 18, 2025 at 9:19 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> The problem presented here is related to NUMA systems and tag-based
> KASAN mode. Getting to it can be explained in the following points:
>
>         1. A new chunk is created with pcpu_create_chunk() and
>            vm_structs are allocated. On systems with one NUMA node only
>            one is allocated, but with more NUMA nodes at least a second
>            one will be allocated too.
>
>         2. chunk->base_addr is assigned the modified value of
>            vms[0]->addr and thus inherits the tag of this allocated
>            structure.
>
>         3. In pcpu_alloc() for each possible cpu pcpu_chunk_addr() is
>            executed which calculates per cpu pointers that correspond to
>            the vms structure addresses. The calculations are based on
>            adding an offset from a table to chunk->base_addr.
>
> Here the problem presents itself since for addresses based on vms[1] and
> up, the tag will be different than the ones based on vms[0] (base_addr).
> The tag mismatch happens and an error is reported.
>
> Reset the base_addr tag, since it will disable tag checks for pointers
> derived arithmetically from base_addr that would inherit its tag.
>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
>  mm/percpu-vm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
> index cd69caf6aa8d..e13750d804f7 100644
> --- a/mm/percpu-vm.c
> +++ b/mm/percpu-vm.c
> @@ -347,7 +347,7 @@ static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
>         }
>
>         chunk->data = vms;
> -       chunk->base_addr = vms[0]->addr - pcpu_group_offsets[0];
> +       chunk->base_addr = kasan_reset_tag(vms[0]->addr) - pcpu_group_offsets[0];

This looks like a generic tags mode bug. I mean that arm64 is also
affected by this.
I assume it just wasn't noticed before because arm64 with multiple
NUMAs are much less common.

With this change tag-mode KASAN won't be able to catch bugus accesses
to pcpu areas.
I'm thinking it would be better to fix this on the pcpu_get_vm_areas()
area side by replacing
this
    for (area = 0; area < nr_vms; area++)
        vms[area]->addr = kasan_unpoison_vmalloc(vms[area]->addr,
                                             vms[area]->size,
KASAN_VMALLOC_PROT_NORMAL);

with something like
    kasan_unpoison_vmap_areas(vms, nr_vms);
which will unpoison all areas using the same tag.

Thoughts?

Re: [PATCH v2 09/14] mm: Pcpu chunk address tag reset

[Andrey Konovalov]

On Thu, Mar 20, 2025 at 6:40 PM Andrey Ryabinin <ryabinin.a.a@gmail.com> wrote:
>
> On Tue, Feb 18, 2025 at 9:19 AM Maciej Wieczor-Retman
> <maciej.wieczor-retman@intel.com> wrote:
> >
> > The problem presented here is related to NUMA systems and tag-based
> > KASAN mode. Getting to it can be explained in the following points:
> >
> >         1. A new chunk is created with pcpu_create_chunk() and
> >            vm_structs are allocated. On systems with one NUMA node only
> >            one is allocated, but with more NUMA nodes at least a second
> >            one will be allocated too.
> >
> >         2. chunk->base_addr is assigned the modified value of
> >            vms[0]->addr and thus inherits the tag of this allocated
> >            structure.
> >
> >         3. In pcpu_alloc() for each possible cpu pcpu_chunk_addr() is
> >            executed which calculates per cpu pointers that correspond to
> >            the vms structure addresses. The calculations are based on
> >            adding an offset from a table to chunk->base_addr.
> >
> > Here the problem presents itself since for addresses based on vms[1] and
> > up, the tag will be different than the ones based on vms[0] (base_addr).
> > The tag mismatch happens and an error is reported.
> >
> > Reset the base_addr tag, since it will disable tag checks for pointers
> > derived arithmetically from base_addr that would inherit its tag.
> >
> > Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> > ---
> >  mm/percpu-vm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
> > index cd69caf6aa8d..e13750d804f7 100644
> > --- a/mm/percpu-vm.c
> > +++ b/mm/percpu-vm.c
> > @@ -347,7 +347,7 @@ static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
> >         }
> >
> >         chunk->data = vms;
> > -       chunk->base_addr = vms[0]->addr - pcpu_group_offsets[0];
> > +       chunk->base_addr = kasan_reset_tag(vms[0]->addr) - pcpu_group_offsets[0];
>
> This looks like a generic tags mode bug. I mean that arm64 is also
> affected by this.
> I assume it just wasn't noticed before because arm64 with multiple
> NUMAs are much less common.
>
> With this change tag-mode KASAN won't be able to catch bugus accesses
> to pcpu areas.
> I'm thinking it would be better to fix this on the pcpu_get_vm_areas()
> area side by replacing
> this
>     for (area = 0; area < nr_vms; area++)
>         vms[area]->addr = kasan_unpoison_vmalloc(vms[area]->addr,
>                                              vms[area]->size,
> KASAN_VMALLOC_PROT_NORMAL);
>
> with something like
>     kasan_unpoison_vmap_areas(vms, nr_vms);
> which will unpoison all areas using the same tag.
>
> Thoughts?

Just a side note: KASAN doesn't have proper handling of the percpu
areas anyway, I even had to remove a related test; see [1] and [2]. So
I think for now we can go with the simplest/consistent solution that
prevents false-positives, whichever that solution is.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=215019
[2] https://bugzilla.kernel.org/show_bug.cgi?id=215758

Re: [PATCH v2 09/14] mm: Pcpu chunk address tag reset

[Maciej Wieczor-Retman]

On 2025-03-20 at 18:39:35 +0100, Andrey Ryabinin wrote:
>On Tue, Feb 18, 2025 at 9:19 AM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> The problem presented here is related to NUMA systems and tag-based
>> KASAN mode. Getting to it can be explained in the following points:
>>
>>         1. A new chunk is created with pcpu_create_chunk() and
>>            vm_structs are allocated. On systems with one NUMA node only
>>            one is allocated, but with more NUMA nodes at least a second
>>            one will be allocated too.
>>
>>         2. chunk->base_addr is assigned the modified value of
>>            vms[0]->addr and thus inherits the tag of this allocated
>>            structure.
>>
>>         3. In pcpu_alloc() for each possible cpu pcpu_chunk_addr() is
>>            executed which calculates per cpu pointers that correspond to
>>            the vms structure addresses. The calculations are based on
>>            adding an offset from a table to chunk->base_addr.
>>
>> Here the problem presents itself since for addresses based on vms[1] and
>> up, the tag will be different than the ones based on vms[0] (base_addr).
>> The tag mismatch happens and an error is reported.
>>
>> Reset the base_addr tag, since it will disable tag checks for pointers
>> derived arithmetically from base_addr that would inherit its tag.
>>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>>  mm/percpu-vm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
>> index cd69caf6aa8d..e13750d804f7 100644
>> --- a/mm/percpu-vm.c
>> +++ b/mm/percpu-vm.c
>> @@ -347,7 +347,7 @@ static struct pcpu_chunk *pcpu_create_chunk(gfp_t gfp)
>>         }
>>
>>         chunk->data = vms;
>> -       chunk->base_addr = vms[0]->addr - pcpu_group_offsets[0];
>> +       chunk->base_addr = kasan_reset_tag(vms[0]->addr) - pcpu_group_offsets[0];
>
>This looks like a generic tags mode bug. I mean that arm64 is also
>affected by this.
>I assume it just wasn't noticed before because arm64 with multiple
>NUMAs are much less common.

That was my assumption as well

>
>With this change tag-mode KASAN won't be able to catch bugus accesses
>to pcpu areas.
>I'm thinking it would be better to fix this on the pcpu_get_vm_areas()
>area side by replacing
>this
>    for (area = 0; area < nr_vms; area++)
>        vms[area]->addr = kasan_unpoison_vmalloc(vms[area]->addr,
>                                             vms[area]->size,
>KASAN_VMALLOC_PROT_NORMAL);
>
>with something like
>    kasan_unpoison_vmap_areas(vms, nr_vms);
>which will unpoison all areas using the same tag.
>
>Thoughts?

I was looking for a solution that would preserve the individual tags for each
area. But so far I didn't come up with anything that would work. I first assumed
that some per-cpu pointers would always be derived from from vms[0]->addr, and
some from vms[1]->addr. For example first half of the cpus would be tied to
vms[0]->addr, second half to vms[1]->addr (since areas are allocated based on
the NUMA layout as far as I know from the docs). But that didn't work and
pointers popped up that were from the second half of the cores but had tags from
the vms[0]->addr.

I think unpoisoning all the areas with the same tag would work, but could it
create issues for out-of-bounds accesses? From my testing the areas are nowhere
near each other but I don't know if that's a given or just by accident?

TLDR: can the areas be adjecent and therefor break OOB checking?

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-02-26 at 16:24:28 +0100, Andrey Konovalov wrote:
>On Wed, Feb 26, 2025 at 12:53 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> After adding
>>         kasan_params += hwasan-instrument-with-calls=0
>> to Makefile.kasan just under
>>         kasan_params += hwasan-mapping-offset=$(KASAN_SHADOW_OFFSET)
>> inline works properly in x86. I looked into assembly and before there were just
>> calls to __hwasan_load/store. After adding the the
>> hwasan-instrument-with-calls=0 I can see no calls and the KASAN offset is now
>> inlined, plus all functions that were previously instrumented now have the
>> kasan_check_range inlined in them.
>>
>> My LLVM investigation lead me to
>>         bool shouldInstrumentWithCalls(const Triple &TargetTriple) {
>>           return optOr(ClInstrumentWithCalls, TargetTriple.getArch() == Triple::x86_64);
>>         }
>> which I assume defaults to "1" on x86? So even with inline mode it doesn't care
>> and still does an outline version.
>
>Ah, indeed. Weird discrepancy between x86 and arm.
>
>Florian, Vitaly, do you recall why this was implemented like this?
>
>To account for this, let's then set hwasan-instrument-with-calls=0
>when CONFIG_KASAN_INLINE is enabled. And also please add a comment
>explaining why this is done.

After adding this option the kernel doesn't want to boot past uncompressing :b

I went into Samuel's clang PR [1] and found there might be one more LShr that
needs changing into AShr [2]? But I'm not very good at clang code. Do you maybe
know if anything else in the clang code could be messing things up?

After changing that LShr to AShr it moves a little further and hangs on some
initmem setup code. Then I thought my KASAN_SHADOW_OFFSET is an issue so I
changed to 4-level paging and the offset to 0xfffffc0000000000 and it moves a
little further and panics on kmem_cache_init. I'll be debugging that further but
just thought I'd ask if you know about something missing from the compiler side?

[1] https://github.com/llvm/llvm-project/pull/103727
[2] https://github.com/SiFiveHolland/llvm-project/blob/up/hwasan-opt/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L995

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Fri, Mar 21, 2025 at 8:21 PM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >To account for this, let's then set hwasan-instrument-with-calls=0
> >when CONFIG_KASAN_INLINE is enabled. And also please add a comment
> >explaining why this is done.
>
> After adding this option the kernel doesn't want to boot past uncompressing :b
>
> I went into Samuel's clang PR [1] and found there might be one more LShr that
> needs changing into AShr [2]? But I'm not very good at clang code. Do you maybe
> know if anything else in the clang code could be messing things up?
>
> After changing that LShr to AShr it moves a little further and hangs on some
> initmem setup code. Then I thought my KASAN_SHADOW_OFFSET is an issue so I
> changed to 4-level paging and the offset to 0xfffffc0000000000 and it moves a
> little further and panics on kmem_cache_init. I'll be debugging that further but
> just thought I'd ask if you know about something missing from the compiler side?
>
> [1] https://github.com/llvm/llvm-project/pull/103727
> [2] https://github.com/SiFiveHolland/llvm-project/blob/up/hwasan-opt/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L995

Hm, I only recall looking at the compiler code when investigating [1].
But as this series points out, [1] can be considered a feature and not
a bug. Other than that, nothing comes to mind.

Thanks!

[1] https://bugzilla.kernel.org/show_bug.cgi?id=218043

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-03-21 at 21:16:12 +0100, Andrey Konovalov wrote:
>On Fri, Mar 21, 2025 at 8:21 PM Maciej Wieczor-Retman
><maciej.wieczor-retman@intel.com> wrote:
>>
>> >To account for this, let's then set hwasan-instrument-with-calls=0
>> >when CONFIG_KASAN_INLINE is enabled. And also please add a comment
>> >explaining why this is done.
>>
>> After adding this option the kernel doesn't want to boot past uncompressing :b
>>
>> I went into Samuel's clang PR [1] and found there might be one more LShr that
>> needs changing into AShr [2]? But I'm not very good at clang code. Do you maybe
>> know if anything else in the clang code could be messing things up?
>>
>> After changing that LShr to AShr it moves a little further and hangs on some
>> initmem setup code. Then I thought my KASAN_SHADOW_OFFSET is an issue so I
>> changed to 4-level paging and the offset to 0xfffffc0000000000 and it moves a
>> little further and panics on kmem_cache_init. I'll be debugging that further but
>> just thought I'd ask if you know about something missing from the compiler side?
>>
>> [1] https://github.com/llvm/llvm-project/pull/103727
>> [2] https://github.com/SiFiveHolland/llvm-project/blob/up/hwasan-opt/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L995
>
>Hm, I only recall looking at the compiler code when investigating [1].
>But as this series points out, [1] can be considered a feature and not
>a bug. Other than that, nothing comes to mind.
>
>Thanks!
>
>[1] https://bugzilla.kernel.org/show_bug.cgi?id=218043

So I assume that if outline mode works, inline mode should be fine as far as
kernel is concerned? If so perhaps it will be more time efficient to post v3 of
this series (once I'm done with kasan_non_canonical_hook() edge cases and
unpoisoning per-cpu vms[areas] with the same tag) and work on the clang side
later / in the meantime.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Maciej Wieczor-Retman]

On 2025-03-24 at 11:43:27 +0100, Maciej Wieczor-Retman wrote:
>On 2025-03-21 at 21:16:12 +0100, Andrey Konovalov wrote:
>>On Fri, Mar 21, 2025 at 8:21 PM Maciej Wieczor-Retman
>><maciej.wieczor-retman@intel.com> wrote:
>>>
>>> >To account for this, let's then set hwasan-instrument-with-calls=0
>>> >when CONFIG_KASAN_INLINE is enabled. And also please add a comment
>>> >explaining why this is done.
>>>
>>> After adding this option the kernel doesn't want to boot past uncompressing :b
>>>
>>> I went into Samuel's clang PR [1] and found there might be one more LShr that
>>> needs changing into AShr [2]? But I'm not very good at clang code. Do you maybe
>>> know if anything else in the clang code could be messing things up?
>>>
>>> After changing that LShr to AShr it moves a little further and hangs on some
>>> initmem setup code. Then I thought my KASAN_SHADOW_OFFSET is an issue so I
>>> changed to 4-level paging and the offset to 0xfffffc0000000000 and it moves a
>>> little further and panics on kmem_cache_init. I'll be debugging that further but
>>> just thought I'd ask if you know about something missing from the compiler side?
>>>
>>> [1] https://github.com/llvm/llvm-project/pull/103727
>>> [2] https://github.com/SiFiveHolland/llvm-project/blob/up/hwasan-opt/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp#L995
>>
>>Hm, I only recall looking at the compiler code when investigating [1].
>>But as this series points out, [1] can be considered a feature and not
>>a bug. Other than that, nothing comes to mind.
>>
>>Thanks!
>>
>>[1] https://bugzilla.kernel.org/show_bug.cgi?id=218043
>
>So I assume that if outline mode works, inline mode should be fine as far as
>kernel is concerned? If so perhaps it will be more time efficient to post v3 of
>this series (once I'm done with kasan_non_canonical_hook() edge cases and
>unpoisoning per-cpu vms[areas] with the same tag) and work on the clang side
>later / in the meantime.

Oh, I guess I also need to add a patch to handle the int3 (X86_TRAP_BP) so
kasan reports show up in inline mode.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v2 13/14] x86: runtime_const used for KASAN_SHADOW_END

[Andrey Konovalov]

On Mon, Mar 24, 2025 at 11:50 AM Maciej Wieczor-Retman
<maciej.wieczor-retman@intel.com> wrote:
>
> >So I assume that if outline mode works, inline mode should be fine as far as
> >kernel is concerned? If so perhaps it will be more time efficient to post v3 of
> >this series (once I'm done with kasan_non_canonical_hook() edge cases and
> >unpoisoning per-cpu vms[areas] with the same tag) and work on the clang side
> >later / in the meantime.

Generally, yes.

The inline mode also might require adding some __no_sanitize_address
annotations. Typically for lower-level function that get messed up by
the inline instrumentation. But the annotations previously added for
the Generic mode would work for SW_TAGS as well, unless SW_TAGS
instrumentation touches some other low-level code.

> Oh, I guess I also need to add a patch to handle the int3 (X86_TRAP_BP) so
> kasan reports show up in inline mode.

Ah, yes, for SW_TAGS, need an appropriate handler here.