On Thu, Feb 20, 2025 at 11:03 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
>
> On Thu, Feb 20, 2025 at 10:53 AM Heiko Carstens <hca@xxxxxxxxxxxxx> wrote:
> >
> > On Thu, Feb 13, 2025 at 02:46:49PM -0800, Suren Baghdasaryan wrote:
> > ...
> > > While this vm_lock replacement does not yet result in a smaller
> > > vm_area_struct (it stays at 256 bytes due to cacheline alignment), it
> > > allows for further size optimization by structure member regrouping
> > > to bring the size of vm_area_struct below 192 bytes.
> > >
> > > Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > > Suggested-by: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > > Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> > > ---
> > > Changes since v9 [1]:
> > > - Use __refcount_inc_not_zero_limited_acquire() in vma_start_read(),
> > > per Hillf Danton
> > > - Refactor vma_assert_locked() to avoid vm_refcnt read when CONFIG_DEBUG_VM=n,
> > > per Mateusz Guzik
> > > - Update changelog, per Wei Yang
> > > - Change vma_start_read() to return EAGAIN if vma got isolated and changed
> > > lock_vma_under_rcu() back to detect this condition, per Wei Yang
> > > - Change VM_BUG_ON_VMA() to WARN_ON_ONCE() when checking vma detached state,
> > > per Lorenzo Stoakes
> > > - Remove Vlastimil's Reviewed-by since code is changed
> >
> > This causes crashes (NULL pointer deref) with linux-next when running
> > the ltp test suite; mtest06 (mmap1) test case.
> >
> > The bug seems to be quite obvious:
> >
> > > @@ -6424,15 +6492,18 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> > > if (!vma)
> > > goto inval;
> > >
> > > - if (!vma_start_read(vma))
> > > - goto inval;
> > > + vma = vma_start_read(vma);
> > > + if (IS_ERR_OR_NULL(vma)) {
> > ^^^^^^^^^^^^^^^^^^^
> > > + /* Check if the VMA got isolated after we found it */
> > > + if (PTR_ERR(vma) == -EAGAIN) {
> > > + vma_end_read(vma);
> > ^^^^^^^^^^^^^^^^
>
> Doh! Thanks for reporting! I'll post a fix shortly.
The fix is posted at:
https://lore.kernel.org/all/20250220200208.323769-1-surenb@xxxxxxxxxx/
Quite embarrassing... I missed removing that extra call. This change
was done only in v10, so never appeared before.
