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

[David Laight <david.laight.linux@gmail.com>]

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?

	David

> 
> v1: https://patch.msgid.link/20260716-verify-rwx-fix-v1-1-d8a04854df06@kernel.org
> ---
>  arch/x86/mm/pat/set_memory.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index d023a40a1e03..cd9ecc8e52f9 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -731,7 +731,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
>  
>  	*level = PG_LEVEL_512G;
>  	*nx |= pgd_flags(*pgd) & _PAGE_NX;
> -	*rw &= pgd_flags(*pgd) & _PAGE_RW;
> +	*rw &= !!(pgd_flags(*pgd) & _PAGE_RW);
>  
>  	p4d = p4d_offset(pgd, address);
>  	if (p4d_none(*p4d))
> @@ -742,7 +742,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
>  
>  	*level = PG_LEVEL_1G;
>  	*nx |= p4d_flags(*p4d) & _PAGE_NX;
> -	*rw &= p4d_flags(*p4d) & _PAGE_RW;
> +	*rw &= !!(p4d_flags(*p4d) & _PAGE_RW);
>  
>  	pud = pud_offset(p4d, address);
>  	if (pud_none(*pud))
> @@ -753,7 +753,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
>  
>  	*level = PG_LEVEL_2M;
>  	*nx |= pud_flags(*pud) & _PAGE_NX;
> -	*rw &= pud_flags(*pud) & _PAGE_RW;
> +	*rw &= !!(pud_flags(*pud) & _PAGE_RW);
>  
>  	pmd = pmd_offset(pud, address);
>  	if (pmd_none(*pmd))
> @@ -764,7 +764,7 @@ pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
>  
>  	*level = PG_LEVEL_4K;
>  	*nx |= pmd_flags(*pmd) & _PAGE_NX;
> -	*rw &= pmd_flags(*pmd) & _PAGE_RW;
> +	*rw &= !!(pmd_flags(*pmd) & _PAGE_RW);
>  
>  	return pte_offset_kernel(pmd, address);
>  }
> 
> ---
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> change-id: 20260716-verify-rwx-fix-3b0bd3e51244
> 
> --
> Sincerely yours,
> Mike.
>