On Tue, Jul 01, 2025 at 05:36:06PM -0700, Sohil Mehta wrote:
> On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> > /*
> > @@ -672,6 +681,12 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs,
> > if (*addr < ~__VIRTUAL_MASK &&
> > *addr + insn.opnd_bytes - 1 > __VIRTUAL_MASK)
> > return GP_NON_CANONICAL;
> > + else if (*addr < ~__VIRTUAL_MASK &&
> > + cpu_feature_enabled(X86_FEATURE_LASS)) {
> > + if (*addr < PAGE_SIZE)
> > + return GP_NULL_POINTER;
> > + return GP_LASS_VIOLATION;
> > + }
>
> The comments above this section of code say:
>
> /*
> * Check that:
> * - the operand is not in the kernel half
> * - the last byte of the operand is not in the user canonical half
> */
>
> They should be updated since we are updating the logic.
Okay.
> Also, below is easier to read than above:
>
> if (*addr < ~__VIRTUAL_MASK) {
>
> if (*addr + insn.opnd_bytes - 1 > __VIRTUAL_MASK)
> return EXC_NON_CANONICAL;
>
> if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> if (*addr < PAGE_SIZE)
> return EXC_NULL_POINTER;
> return EXC_LASS_VIOLATION;
> }
> }
>
> I am wondering if the NULL pointer exception should be made
> unconditional, even if it is unlikely to reach here without LASS. So
> maybe something like this:
>
> if (*addr < ~__VIRTUAL_MASK) {
>
> if (*addr + insn.opnd_bytes - 1 > __VIRTUAL_MASK)
> return EXC_NON_CANONICAL;
>
> if (*addr < PAGE_SIZE)
> return EXC_NULL_POINTER;
>
> if (cpu_feature_enabled(X86_FEATURE_LASS))
> return EXC_LASS_VIOLATION;
> }
That's cleaner.
--
Kiryl Shutsemau / Kirill A. Shutemov
