linux-kernel - Re: [PATCH v8 09/14] x86/mm: LAM compatible non-canonical definition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aWpRwJqjzBxOaRwi@google.com>
Date: Fri, 16 Jan 2026 06:57:04 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Andrey Ryabinin <ryabinin.a.a@...il.com>
Cc: Maciej Wieczor-Retman <m.wieczorretman@...me>, Thomas Gleixner <tglx@...nel.org>, 
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H. Peter Anvin" <hpa@...or.com>, Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>, 
	Alexander Potapenko <glider@...gle.com>, linux-kernel@...r.kernel.org, 
	Paolo Bonzini <pbonzini@...hat.com>, kvm <kvm@...r.kernel.org>
Subject: Re: [PATCH v8 09/14] x86/mm: LAM compatible non-canonical definition

On Fri, Jan 16, 2026, Andrey Ryabinin wrote:
> On 1/12/26 6:28 PM, Maciej Wieczor-Retman wrote:
> > diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> > index bcf5cad3da36..b7940fa49e64 100644
> > --- a/arch/x86/include/asm/page.h
> > +++ b/arch/x86/include/asm/page.h
> > @@ -82,9 +82,22 @@ static __always_inline void *pfn_to_kaddr(unsigned long pfn)
> >  	return __va(pfn << PAGE_SHIFT);
> >  }
> >  
> > +#ifdef CONFIG_KASAN_SW_TAGS
> > +#define CANONICAL_MASK(vaddr_bits) (BIT_ULL(63) | BIT_ULL((vaddr_bits) - 1))
> 
> why is the choice of CANONICAL_MASK() gated at compile time? Shouldn’t this be a
> runtime decision based on whether LAM is enabled or not on the running system?
>  
> > +#else
> > +#define CANONICAL_MASK(vaddr_bits) GENMASK_ULL(63, vaddr_bits)
> > +#endif
> > +
> > +/*
> > + * To make an address canonical either set or clear the bits defined by the
> > + * CANONICAL_MASK(). Clear the bits for userspace addresses if the top address
> > + * bit is a zero. Set the bits for kernel addresses if the top address bit is a
> > + * one.
> > + */
> >  static __always_inline u64 __canonical_address(u64 vaddr, u8 vaddr_bits)
> 
> +Cc KVM

Thanks!

> This is used extensively in KVM code. As far as I can tell, it may be used to
> determine whether a guest virtual address is canonical or not.

Yep, KVM uses this both to check canonical addresses and to force a canonical
address (Intel and AMD disagree on the MSR_IA32_SYSENTER_{EIP,ESP} semantics in
64-bit mode) for guest addresses.  This change will break KVM badly if KASAN_SW_TAGS=y.

> case, the result should depend on whether LAM is enabled for the guest, not
> the host (and certainly not a host's compile-time option).

Ya, KVM could roll its own versions, but IMO these super low level helpers should
do exactly what they say.  E.g. at a glance, I'm not sure pt_event_addr_filters_sync()
should be subjected to KASAN_SW_TAGS either.  If that's true, then AFAICT the
_only_ code that wants the LAM-aware behavior is copy_from_kernel_nofault_allowed(),
so maybe just handle it there?  Not sure that's a great long-term maintenance
story either though.