For an address to be canonical it has to have its top bits equal to each
other. The number of bits depends on the paging level and whether
they're supposed to be ones or zeroes depends on whether the address
points to kernel or user space.

With Linear Address Masking (LAM) enabled, the definition of linear
address canonicality is modified. Not all of the previously required
bits need to be equal, only the first and last from the previously equal
bitmask. So for example a 5-level paging kernel address needs to have
bits [63] and [56] set.

Add separate __canonical_address() implementation for
CONFIG_KASAN_SW_TAGS since it's the only thing right now that enables
LAM for kernel addresses (LAM_SUP bit in CR4).

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

 arch/x86/include/asm/page.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index bcf5cad3da36..a83f23a71f35 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -82,10 +82,20 @@ static __always_inline void *pfn_to_kaddr(unsigned long pfn)
 	return __va(pfn << PAGE_SHIFT);
 }
 
+/*
+ * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
+ */
+#ifdef CONFIG_KASAN_SW_TAGS
+static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
+{
+	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
+}
+#else
 static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
 {
 	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
 }
+#endif
 
 static __always_inline u64 __is_canonical_address(u64 vaddr, u8 vaddr_bits)
 {
-- 
2.50.1

On 8/25/25 13:24, Maciej Wieczor-Retman wrote:
> +/*
> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
> + */
> +#ifdef CONFIG_KASAN_SW_TAGS
> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
> +{
> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
> +}
> +#else
>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>  {
>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>  }
> +#endif

This is the kind of thing that's bound to break. Could we distill it
down to something simpler, perhaps?

In the end, the canonical enforcement mask is the thing that's changing.
So perhaps it should be all common code except for the mask definition:

#ifdef CONFIG_KASAN_SW_TAGS
#define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL(vaddr_bits-1))
#else
#define CANONICAL_MASK(vaddr_bits) GENMASK_UL(63, vaddr_bits)
#endif

(modulo off-by-one bugs ;)

Then the canonical check itself becomes something like:

	unsigned long cmask = CANONICAL_MASK(vaddr_bits);
	return (vaddr & mask) == mask;

That, to me, is the most straightforward way to do it.

I don't see it addressed in the cover letter, but what happens when a
CONFIG_KASAN_SW_TAGS=y kernel is booted on non-LAM hardware?

On 2025-08-25 at 14:36:35 -0700, Dave Hansen wrote:
>On 8/25/25 13:24, Maciej Wieczor-Retman wrote:
>> +/*
>> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
>> + */
>> +#ifdef CONFIG_KASAN_SW_TAGS
>> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>> +{
>> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
>> +}
>> +#else
>>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>  {
>>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>>  }
>> +#endif
>
>This is the kind of thing that's bound to break. Could we distill it
>down to something simpler, perhaps?
>
>In the end, the canonical enforcement mask is the thing that's changing.
>So perhaps it should be all common code except for the mask definition:
>
>#ifdef CONFIG_KASAN_SW_TAGS
>#define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL(vaddr_bits-1))
>#else
>#define CANONICAL_MASK(vaddr_bits) GENMASK_UL(63, vaddr_bits)
>#endif
>
>(modulo off-by-one bugs ;)
>
>Then the canonical check itself becomes something like:
>
>	unsigned long cmask = CANONICAL_MASK(vaddr_bits);
>	return (vaddr & mask) == mask;
>
>That, to me, is the most straightforward way to do it.

Thanks, I'll try something like this. I will also have to investigate what
Samuel brought up that KVM possibly wants to pass user addresses to this
function as well.

>
>I don't see it addressed in the cover letter, but what happens when a
>CONFIG_KASAN_SW_TAGS=y kernel is booted on non-LAM hardware?

That's a good point, I need to add it to the cover letter. On non-LAM hardware
the kernel just doesn't boot. Disabling KASAN in runtime on unsupported hardware
isn't that difficult in outline mode, but I'm not sure it can work in inline
mode (where checks into shadow memory are just pasted into code by the
compiler).

Since for now there is no compiler support for the inline mode anyway, I'll try to
disable KASAN on non-LAM hardware in runtime.

-- 
Kind regards
Maciej Wieczór-Retman

Hi Maciej,

On 2025-08-26 3:08 AM, Maciej Wieczor-Retman wrote:
> On 2025-08-25 at 14:36:35 -0700, Dave Hansen wrote:
>> On 8/25/25 13:24, Maciej Wieczor-Retman wrote:
>>> +/*
>>> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
>>> + */
>>> +#ifdef CONFIG_KASAN_SW_TAGS
>>> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>> +{
>>> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
>>> +}
>>> +#else
>>>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>>  {
>>>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>>>  }
>>> +#endif
>>
>> This is the kind of thing that's bound to break. Could we distill it
>> down to something simpler, perhaps?
>>
>> In the end, the canonical enforcement mask is the thing that's changing.
>> So perhaps it should be all common code except for the mask definition:
>>
>> #ifdef CONFIG_KASAN_SW_TAGS
>> #define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL(vaddr_bits-1))
>> #else
>> #define CANONICAL_MASK(vaddr_bits) GENMASK_UL(63, vaddr_bits)
>> #endif
>>
>> (modulo off-by-one bugs ;)
>>
>> Then the canonical check itself becomes something like:
>>
>> 	unsigned long cmask = CANONICAL_MASK(vaddr_bits);
>> 	return (vaddr & mask) == mask;
>>
>> That, to me, is the most straightforward way to do it.
> 
> Thanks, I'll try something like this. I will also have to investigate what
> Samuel brought up that KVM possibly wants to pass user addresses to this
> function as well.
> 
>>
>> I don't see it addressed in the cover letter, but what happens when a
>> CONFIG_KASAN_SW_TAGS=y kernel is booted on non-LAM hardware?
> 
> That's a good point, I need to add it to the cover letter. On non-LAM hardware
> the kernel just doesn't boot. Disabling KASAN in runtime on unsupported hardware
> isn't that difficult in outline mode, but I'm not sure it can work in inline
> mode (where checks into shadow memory are just pasted into code by the
> compiler).

On RISC-V at least, I was able to run inline mode with missing hardware support.
The shadow memory is still allocated, so the inline tag checks do not fault. And
with a patch to make kasan_enabled() return false[1], all pointers remain
canonical (they match the MatchAllTag), so the inline tag checks all succeed.

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

Regards,
Samuel

> Since for now there is no compiler support for the inline mode anyway, I'll try to
> disable KASAN on non-LAM hardware in runtime.
>

On 2025-08-26 at 19:46:19 -0500, Samuel Holland wrote:
>Hi Maciej,
>
>On 2025-08-26 3:08 AM, Maciej Wieczor-Retman wrote:
>> On 2025-08-25 at 14:36:35 -0700, Dave Hansen wrote:
>>> On 8/25/25 13:24, Maciej Wieczor-Retman wrote:
>>>> +/*
>>>> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
>>>> + */
>>>> +#ifdef CONFIG_KASAN_SW_TAGS
>>>> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>>> +{
>>>> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
>>>> +}
>>>> +#else
>>>>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>>>  {
>>>>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>>>>  }
>>>> +#endif
>>>
>>> This is the kind of thing that's bound to break. Could we distill it
>>> down to something simpler, perhaps?
>>>
>>> In the end, the canonical enforcement mask is the thing that's changing.
>>> So perhaps it should be all common code except for the mask definition:
>>>
>>> #ifdef CONFIG_KASAN_SW_TAGS
>>> #define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL(vaddr_bits-1))
>>> #else
>>> #define CANONICAL_MASK(vaddr_bits) GENMASK_UL(63, vaddr_bits)
>>> #endif
>>>
>>> (modulo off-by-one bugs ;)
>>>
>>> Then the canonical check itself becomes something like:
>>>
>>> 	unsigned long cmask = CANONICAL_MASK(vaddr_bits);
>>> 	return (vaddr & mask) == mask;
>>>
>>> That, to me, is the most straightforward way to do it.
>> 
>> Thanks, I'll try something like this. I will also have to investigate what
>> Samuel brought up that KVM possibly wants to pass user addresses to this
>> function as well.
>> 
>>>
>>> I don't see it addressed in the cover letter, but what happens when a
>>> CONFIG_KASAN_SW_TAGS=y kernel is booted on non-LAM hardware?
>> 
>> That's a good point, I need to add it to the cover letter. On non-LAM hardware
>> the kernel just doesn't boot. Disabling KASAN in runtime on unsupported hardware
>> isn't that difficult in outline mode, but I'm not sure it can work in inline
>> mode (where checks into shadow memory are just pasted into code by the
>> compiler).
>
>On RISC-V at least, I was able to run inline mode with missing hardware support.
>The shadow memory is still allocated, so the inline tag checks do not fault. And
>with a patch to make kasan_enabled() return false[1], all pointers remain
>canonical (they match the MatchAllTag), so the inline tag checks all succeed.
>
>[1]:
>https://lore.kernel.org/linux-riscv/20241022015913.3524425-3-samuel.holland@sifive.com/

Thanks, that should work :)

I'll test it and apply to the series.

>
>Regards,
>Samuel
>
>> Since for now there is no compiler support for the inline mode anyway, I'll try to
>> disable KASAN on non-LAM hardware in runtime.
>> 
>

-- 
Kind regards
Maciej Wieczór-Retman

Hi Maciej,

On 2025-08-25 3:24 PM, Maciej Wieczor-Retman wrote:
> For an address to be canonical it has to have its top bits equal to each
> other. The number of bits depends on the paging level and whether
> they're supposed to be ones or zeroes depends on whether the address
> points to kernel or user space.
> 
> With Linear Address Masking (LAM) enabled, the definition of linear
> address canonicality is modified. Not all of the previously required
> bits need to be equal, only the first and last from the previously equal
> bitmask. So for example a 5-level paging kernel address needs to have
> bits [63] and [56] set.
> 
> Add separate __canonical_address() implementation for
> CONFIG_KASAN_SW_TAGS since it's the only thing right now that enables
> LAM for kernel addresses (LAM_SUP bit in CR4).
> 
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v4:
> - Add patch to the series.
> 
>  arch/x86/include/asm/page.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> index bcf5cad3da36..a83f23a71f35 100644
> --- a/arch/x86/include/asm/page.h
> +++ b/arch/x86/include/asm/page.h
> @@ -82,10 +82,20 @@ static __always_inline void *pfn_to_kaddr(unsigned long pfn)
>  	return __va(pfn << PAGE_SHIFT);
>  }
>  
> +/*
> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
> + */
> +#ifdef CONFIG_KASAN_SW_TAGS
> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
> +{
> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
> +}
> +#else
>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>  {
>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>  }
> +#endif

These two implementations have different semantics. The new function works only
on kernel addresses, whereas the existing one works on user addresses as well.
It looks like at least KVM's use of __is_canonical_address() expects the
function to work with user addresses.

Regards,
Samuel

>  
>  static __always_inline u64 __is_canonical_address(u64 vaddr, u8 vaddr_bits)
>  {

On 2025-08-25 at 15:59:46 -0500, Samuel Holland wrote:
>Hi Maciej,
>
>On 2025-08-25 3:24 PM, Maciej Wieczor-Retman wrote:
>> For an address to be canonical it has to have its top bits equal to each
>> other. The number of bits depends on the paging level and whether
>> they're supposed to be ones or zeroes depends on whether the address
>> points to kernel or user space.
>> 
>> With Linear Address Masking (LAM) enabled, the definition of linear
>> address canonicality is modified. Not all of the previously required
>> bits need to be equal, only the first and last from the previously equal
>> bitmask. So for example a 5-level paging kernel address needs to have
>> bits [63] and [56] set.
>> 
>> Add separate __canonical_address() implementation for
>> CONFIG_KASAN_SW_TAGS since it's the only thing right now that enables
>> LAM for kernel addresses (LAM_SUP bit in CR4).
>> 
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v4:
>> - Add patch to the series.
>> 
>>  arch/x86/include/asm/page.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
>> index bcf5cad3da36..a83f23a71f35 100644
>> --- a/arch/x86/include/asm/page.h
>> +++ b/arch/x86/include/asm/page.h
>> @@ -82,10 +82,20 @@ static __always_inline void *pfn_to_kaddr(unsigned long pfn)
>>  	return __va(pfn << PAGE_SHIFT);
>>  }
>>  
>> +/*
>> + * CONFIG_KASAN_SW_TAGS requires LAM which changes the canonicality checks.
>> + */
>> +#ifdef CONFIG_KASAN_SW_TAGS
>> +static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>> +{
>> +	return (vaddr | BIT_ULL(63) | BIT_ULL(vaddr_bits - 1));
>> +}
>> +#else
>>  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
>>  {
>>  	return ((s64)vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits);
>>  }
>> +#endif
>
>These two implementations have different semantics. The new function works only
>on kernel addresses, whereas the existing one works on user addresses as well.
>It looks like at least KVM's use of __is_canonical_address() expects the
>function to work with user addresses.

Thanks for noticing that, I'll think of a way to make it work for user addresses
too :)

>
>Regards,
>Samuel
>
>>  
>>  static __always_inline u64 __is_canonical_address(u64 vaddr, u8 vaddr_bits)
>>  {
>

-- 
Kind regards
Maciej Wieczór-Retman