Re: [bug report] arm64/mm: Clear PXX_TYPE_MASK and set PXD_TYPE_SECT in [pmd|pud]_mkhuge()

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Mar 14, 2025 at 05:52:04PM +0000, Catalin Marinas wrote:
> On Fri, Mar 14, 2025 at 01:54:01PM +0300, Dan Carpenter wrote:
> > Hello Anshuman Khandual,
> > 
> > Commit 1601df9e366e ("arm64/mm: Clear PXX_TYPE_MASK and set
> > PXD_TYPE_SECT in [pmd|pud]_mkhuge()") from Feb 21, 2025 (linux-next),
> > leads to the following (unpublished) Smatch static checker warning:
> > 
> > arch/arm64/include/asm/pgtable.h:587 pmd_mkhuge() warn: odd binop '0x1 & 0xfffffffffffffffe'
> > arch/arm64/include/asm/pgtable.h:626 pud_mkhuge() warn: odd binop '0x1 & 0xfffffffffffffffe'
> > 
> > arch/arm64/include/asm/pgtable.h
> >     579 static inline pmd_t pmd_mkhuge(pmd_t pmd)
> >     580 {
> >     581         /*
> >     582          * It's possible that the pmd is present-invalid on entry
> >     583          * and in that case it needs to remain present-invalid on
> >     584          * exit. So ensure the VALID bit does not get modified.
> >     585          */
> >     586         pmdval_t mask = PMD_TYPE_MASK & ~PTE_VALID;
> > --> 587         pmdval_t val = PMD_TYPE_SECT & ~PTE_VALID;
> > 
> > This is "1 & ~1".  I see the comment, but I'm too stupid to know even
> > after reading he comment whether it's intentional or not.  :P
> > 
> >     588 
> >     589         return __pmd((pmd_val(pmd) & ~mask) | val);
> >     590 }
> 
> Prior to the above commit, the code was:
> 
> #define pmd_mkhuge(pmd)			(__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
> 
> So just clearing a bit. That's why the reworked code has val 0 in the
> current configuration. This may change with support for 128-bit PTEs
> though, different format.
> 
> The comment is about preserving the PTE_VALID bit in the original pmd
> value, so it's masked out of both mask and val.
> 
> It's good that smatch catches these though. Any way to mark a false
> positive?

In the kernel, all old warnings are false positives.  We're good at
fixing things.  People can look it up on lore and find this thread
if they have questions.

regards,
dan carpenter