From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Liam Howlett <liam.howlett@oracle.com>
Cc: "maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"syzbot+0d2014e4da2ccced5b41@syzkaller.appspotmail.com"
<syzbot+0d2014e4da2ccced5b41@syzkaller.appspotmail.com>
Subject: Re: [PATCH] fs/userfaultfd: Fix maple tree iterator in userfaultfd_unregister()
Date: Mon, 7 Nov 2022 13:13:43 -0800 [thread overview]
Message-ID: <CAHk-=wg9WQXBGkNdKD2bqocnN73rDswuWsavBB7T-tekykEn_A@mail.gmail.com> (raw)
In-Reply-To: <20221107201121.1169273-1-Liam.Howlett@oracle.com>
On Mon, Nov 7, 2022 at 12:12 PM Liam Howlett <liam.howlett@oracle.com> wrote:
>
> When iterating the VMAs, the maple state needs to be invalidated if the
> tree is modified by a split or merge to ensure the maple tree node
> contained in the maple state is still valid. These invalidations were
> missed, so add them to the paths which alter the tree.
I have applied this as an obvious fix, but I would *really* want to
also see longer-term
- I'd really like the 'mas' operations to have 'vma' specializations
that get the type safety right
- that mas_pause() name is horrible, please let's just fix it to
something sensible in this context
- moving the iterator invalidation into split_vma() and vma_merge()
or at least have some other means of not having these mistakes
From what I can tell, things like mprotect() and mlock() - end up not
using the iterator at all because of this issue. Instead they seem to
just do
vma = find_vma(current->mm, prev->vm_end);
despite having actually started out with the whole iterator state.
Except for 'apply_mlockall_flags()' that randomly does end up usign
the iterator (and has that mas_pause() as a result).
So it would make *sense* to have "mlock_fixup()" take a MA_STATE
instead of "vma, &prev" as arguments, but it doesn't.
I dunno. Maybe there's some other reason for this very non-intuitive
mix of "sometimes iterators, sometimes not, and always horrible
naming".
Linus
next prev parent reply other threads:[~2022-11-07 21:18 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-07 20:11 [PATCH] fs/userfaultfd: Fix maple tree iterator in userfaultfd_unregister() Liam Howlett
2022-11-07 21:13 ` Linus Torvalds [this message]
2022-11-07 22:04 ` Liam 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='CAHk-=wg9WQXBGkNdKD2bqocnN73rDswuWsavBB7T-tekykEn_A@mail.gmail.com' \
--to=torvalds@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=liam.howlett@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maple-tree@lists.infradead.org \
--cc=syzbot+0d2014e4da2ccced5b41@syzkaller.appspotmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.