On Tue, 11 Jul 2023, Dan Carpenter wrote:
> Hello Hugh Dickins,
>
> The patch 0d940a9b270b: "mm/pgtable: allow pte_offset_map[_lock]() to
> fail" from Jun 8, 2023, leads to the following Smatch static checker
> warning:
>
> mm/userfaultfd.c:321 mfill_atomic_pte_poison()
> error: uninitialized symbol 'ptl'.
In mm-unstable or linux-next: not a problem in 6.5-rc.
Thanks, Dan: yes.
A little unfair to blame my patch for code that went in later, but it's
a good reminder that I do need to keep checking for new usages of
pte_offset_map[_lock](), and this is the only one so far.
Great that Smatch is helping me with that.
Fix to the guilty patch follows.
Hugh
>
> mm/userfaultfd.c
> 292 static int mfill_atomic_pte_poison(pmd_t *dst_pmd,
> 293 struct vm_area_struct *dst_vma,
> 294 unsigned long dst_addr,
> 295 uffd_flags_t flags)
> 296 {
> 297 int ret;
> 298 struct mm_struct *dst_mm = dst_vma->vm_mm;
> 299 pte_t _dst_pte, *dst_pte;
> 300 spinlock_t *ptl;
> 301
> 302 _dst_pte = make_pte_marker(PTE_MARKER_POISONED);
> 303 dst_pte = pte_offset_map_lock(dst_mm, dst_pmd, dst_addr, &ptl);
> ^^^^
> The __pte_offset_map_lock() function does not initialize ptl if it
> returns NULL.
>
> 304
> 305 if (mfill_file_over_size(dst_vma, dst_addr)) {
> 306 ret = -EFAULT;
> 307 goto out_unlock;
> 308 }
> 309
> 310 ret = -EEXIST;
> 311 /* Refuse to overwrite any PTE, even a PTE marker (e.g. UFFD WP). */
> 312 if (!pte_none(*dst_pte))
> 313 goto out_unlock;
> 314
> 315 set_pte_at(dst_mm, dst_addr, dst_pte, _dst_pte);
> 316
> 317 /* No need to invalidate - it was non-present before */
> 318 update_mmu_cache(dst_vma, dst_addr, dst_pte);
> 319 ret = 0;
> 320 out_unlock:
> --> 321 pte_unmap_unlock(dst_pte, ptl);
> 322 return ret;
> 323 }
>
> regards,
> dan carpenter
