From: "Paul E. McKenney" <paulmck@kernel.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
"Liam R. Howlett" <Liam.Howlett@oracle.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Suren Baghdasaryan <surenb@google.com>,
Vlastimil Babka <vbabka@suse.cz>,
Shakeel Butt <shakeel.butt@linux.dev>,
Jann Horn <jannh@google.com>,
stable@vger.kernel.org,
syzbot+131f9eb2b5807573275c@syzkaller.appspotmail.com
Subject: Re: [PATCH] mm/mmap_lock: Reset maple state on lock_vma_under_rcu() retry
Date: Wed, 12 Nov 2025 17:27:22 -0800 [thread overview]
Message-ID: <d9e181fe-e3d8-427a-8323-ea979f5a02ad@paulmck-laptop> (raw)
In-Reply-To: <aRUggwAQJsnQV_07@casper.infradead.org>
On Thu, Nov 13, 2025 at 12:04:19AM +0000, Matthew Wilcox wrote:
> On Wed, Nov 12, 2025 at 03:06:38PM +0000, Lorenzo Stoakes wrote:
> > > Any time the rcu read lock is dropped, the maple state must be
> > > invalidated. Resetting the address and state to MA_START is the safest
> > > course of action, which will result in the next operation starting from
> > > the top of the tree.
> >
> > Since we all missed it I do wonder if we need some super clear comment
> > saying 'hey if you drop + re-acquire RCU lock you MUST revalidate mas state
> > by doing 'blah'.
>
> I mean, this really isn't an RCU thing. This is also bad:
>
> spin_lock(a);
> p = *q;
> spin_unlock(a);
> spin_lock(a);
> b = *p;
>
> p could have been freed while you didn't hold lock a. Detecting this
> kind of thing needs compiler assistence (ie Rust) to let you know that
> you don't have the right to do that any more.
While in no way denigrating Rust's compile-time detection of this sort
of thing, use of KASAN combined with CONFIG_RCU_STRICT_GRACE_PERIOD=y
(which restricts you to four CPUs) can sometimes help.
> > I think one source of confusion for me with maple tree operations is - what
> > to do if we are in a position where some kind of reset is needed?
> >
> > So even if I'd realised 'aha we need to reset this' it wouldn't be obvious
> > to me that we ought to set to the address.
>
> I think that's a separate problem.
>
> > > +++ b/mm/mmap_lock.c
> > > @@ -257,6 +257,7 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> > > if (PTR_ERR(vma) == -EAGAIN) {
> > > count_vm_vma_lock_event(VMA_LOCK_MISS);
> > > /* The area was replaced with another one */
> > > + mas_set(&mas, address);
> >
> > I wonder if we could detect that the RCU lock was released (+ reacquired) in
> > mas_walk() in a debug mode, like CONFIG_VM_DEBUG_MAPLE_TREE?
>
> Dropping and reacquiring the RCU read lock should have been a big red
> flag. I didn't have time to review the patches, but if I had, I would
> have suggested passing the mas down to the routine that drops the rcu
> read lock so it can be invalidated before dropping the readlock.
There has been some academic efforts to check for RCU-protected pointers
leaking from one RCU read-side critical section to another, but nothing
useful has come from this. :-/
But rcu_pointer_handoff() and unrcu_pointer() are intended not only for
documentation, but also to suppress the inevitable false positives should
anyone figure out how to detect leaking of RCU-protected pointers.
Thanx, Paul
next prev parent reply other threads:[~2025-11-13 1:27 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-11 21:56 Liam R. Howlett
2025-11-11 22:18 ` Vlastimil Babka
2025-11-12 0:10 ` Suren Baghdasaryan
2025-11-12 0:19 ` Liam R. Howlett
2025-11-12 0:45 ` Suren Baghdasaryan
2025-11-12 2:18 ` Liam R. Howlett
2025-11-12 20:24 ` Andrew Morton
2025-11-12 15:06 ` Lorenzo Stoakes
2025-11-12 16:10 ` Liam R. Howlett
2025-11-13 15:15 ` Lorenzo Stoakes
2025-11-13 0:04 ` Matthew Wilcox
2025-11-13 1:27 ` Paul E. McKenney [this message]
2025-11-13 11:05 ` Lorenzo Stoakes
2025-11-21 9:08 ` Vlastimil Babka
2025-11-21 16:52 ` Paul E. McKenney
2025-11-13 10:45 ` Lorenzo Stoakes
2025-11-13 17:28 ` Liam R. Howlett
2025-11-14 11:51 ` Lorenzo Stoakes
2025-11-14 17:18 ` Liam R. Howlett
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d9e181fe-e3d8-427a-8323-ea979f5a02ad@paulmck-laptop \
--to=paulmck@kernel.org \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lorenzo.stoakes@oracle.com \
--cc=shakeel.butt@linux.dev \
--cc=stable@vger.kernel.org \
--cc=surenb@google.com \
--cc=syzbot+131f9eb2b5807573275c@syzkaller.appspotmail.com \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox