* [bug report] mm/pgtable: allow pte_offset_map[_lock]() to fail
@ 2023-07-11 7:25 Dan Carpenter
2023-07-12 1:23 ` Hugh Dickins
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-07-11 7:25 UTC (permalink / raw)
To: hughd; +Cc: linux-mm
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'.
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] mm/pgtable: allow pte_offset_map[_lock]() to fail
2023-07-11 7:25 [bug report] mm/pgtable: allow pte_offset_map[_lock]() to fail Dan Carpenter
@ 2023-07-12 1:23 ` Hugh Dickins
2023-07-12 6:50 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2023-07-12 1:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Hugh Dickins, Andrew Morton, Axel Rasmussen, Peter Xu, linux-mm
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] mm/pgtable: allow pte_offset_map[_lock]() to fail
2023-07-12 1:23 ` Hugh Dickins
@ 2023-07-12 6:50 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2023-07-12 6:50 UTC (permalink / raw)
To: Hugh Dickins; +Cc: Andrew Morton, Axel Rasmussen, Peter Xu, linux-mm
On Tue, Jul 11, 2023 at 06:23:07PM -0700, Hugh Dickins wrote:
> 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
Sorry, Hugh!
I just saw your name in the git log and knew that you were a person who
knows how to get things done so I didn't bother looking further than
that!
I will add that it's on linux-next to my bug report template.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-07-12 9:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11 7:25 [bug report] mm/pgtable: allow pte_offset_map[_lock]() to fail Dan Carpenter
2023-07-12 1:23 ` Hugh Dickins
2023-07-12 6:50 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox