[bug report] mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)

[bug report] mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)

[Dan Carpenter]

Hello Qi Zheng,

Commit e3aafd2d3551 ("mm: pgtable: reclaim empty PTE page in
madvise(MADV_DONTNEED)") from Dec 4, 2024 (linux-next), leads to the
following Smatch static checker warning:

	mm/pt_reclaim.c:69 try_to_free_pte()
	error: uninitialized symbol 'ptl'.

mm/pt_reclaim.c
    35 void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, unsigned long addr,
    36                      struct mmu_gather *tlb)
    37 {
    38         pmd_t pmdval;
    39         spinlock_t *pml, *ptl;
    40         pte_t *start_pte, *pte;
    41         int i;
    42 
    43         pml = pmd_lock(mm, pmd);
    44         start_pte = pte_offset_map_rw_nolock(mm, pmd, addr, &pmdval, &ptl);
    45         if (!start_pte)
    46                 goto out_ptl;

ptl is uninitialized on error.

    47         if (ptl != pml)
    48                 spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
    49 
    50         /* Check if it is empty PTE page */
    51         for (i = 0, pte = start_pte; i < PTRS_PER_PTE; i++, pte++) {
    52                 if (!pte_none(ptep_get(pte)))
    53                         goto out_ptl;
    54         }
    55         pte_unmap(start_pte);
    56 
    57         pmd_clear(pmd);
    58 
    59         if (ptl != pml)
    60                 spin_unlock(ptl);
    61         spin_unlock(pml);
    62 
    63         free_pte(mm, addr, tlb, pmdval);
    64 
    65         return;
    66 out_ptl:
    67         if (start_pte)
    68                 pte_unmap_unlock(start_pte, ptl);
--> 69         if (ptl != pml)
                   ^^^
Uninitialized variable

    70                 spin_unlock(pml);
    71 }

regards,
dan carpenter

Re: [bug report] mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)

[Qi Zheng]

On 2024/12/6 18:46, Dan Carpenter wrote:
> Hello Qi Zheng,
> 
> Commit e3aafd2d3551 ("mm: pgtable: reclaim empty PTE page in
> madvise(MADV_DONTNEED)") from Dec 4, 2024 (linux-next), leads to the
> following Smatch static checker warning:
> 
> 	mm/pt_reclaim.c:69 try_to_free_pte()
> 	error: uninitialized symbol 'ptl'.

Thanks for reporting this! Will fix it like below:

diff --git a/mm/pt_reclaim.c b/mm/pt_reclaim.c
index 6540a3115dde8..7e9455a18aae7 100644
--- a/mm/pt_reclaim.c
+++ b/mm/pt_reclaim.c
@@ -36,7 +36,7 @@ void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, 
unsigned long addr,
                      struct mmu_gather *tlb)
  {
         pmd_t pmdval;
-       spinlock_t *pml, *ptl;
+       spinlock_t *pml, *ptl = NULL;
         pte_t *start_pte, *pte;
         int i;

> 
> mm/pt_reclaim.c
>      35 void try_to_free_pte(struct mm_struct *mm, pmd_t *pmd, unsigned long addr,
>      36                      struct mmu_gather *tlb)
>      37 {
>      38         pmd_t pmdval;
>      39         spinlock_t *pml, *ptl;
>      40         pte_t *start_pte, *pte;
>      41         int i;
>      42
>      43         pml = pmd_lock(mm, pmd);
>      44         start_pte = pte_offset_map_rw_nolock(mm, pmd, addr, &pmdval, &ptl);
>      45         if (!start_pte)
>      46                 goto out_ptl;
> 
> ptl is uninitialized on error.
> 
>      47         if (ptl != pml)
>      48                 spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
>      49
>      50         /* Check if it is empty PTE page */
>      51         for (i = 0, pte = start_pte; i < PTRS_PER_PTE; i++, pte++) {
>      52                 if (!pte_none(ptep_get(pte)))
>      53                         goto out_ptl;
>      54         }
>      55         pte_unmap(start_pte);
>      56
>      57         pmd_clear(pmd);
>      58
>      59         if (ptl != pml)
>      60                 spin_unlock(ptl);
>      61         spin_unlock(pml);
>      62
>      63         free_pte(mm, addr, tlb, pmdval);
>      64
>      65         return;
>      66 out_ptl:
>      67         if (start_pte)
>      68                 pte_unmap_unlock(start_pte, ptl);
> --> 69         if (ptl != pml)
>                     ^^^
> Uninitialized variable
> 
>      70                 spin_unlock(pml);
>      71 }
> 
> regards,
> dan carpenter