Re: [PATCH v2] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()

[Mike Rapoport <rppt@kernel.org>]

On Fri, Jul 17, 2026 at 11:32:09AM +0100, David Laight wrote:
> On Fri, 17 Jul 2026 12:41:43 +0300
> "Mike Rapoport (Microsoft)" <rppt@kernel.org> wrote:
> 
> > lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
> > the walked page table levels so that verify_rwx() can detect mappings that
> > are both writable and executable.
> > 
> > The RW bits are folded into a bool with
> > 
> > 	rw &= pXd_flags(*pXd) & _PAGE_RW;
> > 
> > but _PAGE_RW is 0x2. So consider the accumulation line:
> > 
> >         rw &= pXd_flags(*pXd) & _PAGE_RW;
> > 
> > where rw=0x1 and the right side evaluates down to 0x2. It'll end up doing:
> > 
> >         rw = 0x1 & 0x2
> > 
> > and rw always ends up 0.
> > 
> > This way rw becomes false at the first level walked, regardless of the
> > actual permissions, and verify_rwx() treats every mapping as non-writable
> > and never reports a W^X violation.
> > 
> > Add double negation to the right side to normalize the _PAGE_RW flag to
> > 0 or 1.
> > 
> > Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
> > Assisted-by: Copilot:claude-opus-4.8
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > Reviewed-by: Juergen Gross <jgross@suse.com>
> > ---
> > v2 changes:
> > * use a simpler version that uses !! to ensure the comparison result is
> >   0 or 1
> 
> Isn't this slower (and probably larger) because of the repeated references
> to the parameter passed by reference?

Yes, it is, but Dave objected micro-optimization:

https://lore.kernel.org/all/adecc483-ca7e-4c03-ab35-88a0400a4867@intel.com/

and there is no measurable difference in BPF program load times.
 
> 	David

-- 
Sincerely yours,
Mike.