linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Suren Baghdasaryan <surenb@google.com>
Cc: sidhartha.kumar@oracle.com, maple-tree@lists.infradead.org,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Zhaoyang Huang <zhaoyang.huang@unisoc.com>,
	Hailong Liu <hailong.liu@oppo.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	"zhangpeng . 00 @ bytedance . com" <zhangpeng.00@bytedance.com>,
	Steve Kang <Steve.Kang@unisoc.com>,
	Matthew Wilcox <willy@infradead.org>
Subject: Re: [RFC PATCH v6.6] maple_tree: Fix mas_prealloc() reset
Date: Mon, 28 Apr 2025 20:19:52 -0400	[thread overview]
Message-ID: <2tf7t2tjwsl7oij255uwsyxvmnxg54vsa2rlogkyrevlv2tr5b@c4kcoju752y7> (raw)
In-Reply-To: <CAJuCfpF9aBvTw-r-CMM4+BgbAKCePZpR7_H4ouifWDHDiYQ8Qg@mail.gmail.com>

* Suren Baghdasaryan <surenb@google.com> [250428 17:08]:
> On Mon, Apr 28, 2025 at 12:02 PM Liam R. Howlett
> <Liam.Howlett@oracle.com> wrote:
> >
> > A previously used maple state may not be in the correct state for the
> > preallocation to correctly calculate the number of nodes necessary for the
> > configured store operation.
> >
> > The user visible effect of which is warning that there are no nodes
> > allocated followed by a crash when there is a null pointer dereference
> > shortly after.
> >
> > The NULL pointer dereference has been reported to happen when a vma
> > iterator is used to preallocate after walking to a leaf node but then
> > requesting to preallocate for a store across node boundaries (in v6.6.
> > of the kernel).  The altered range means that there may not been enough
> > nodes as the maple state has been incorrectly configured.  A critical
> > step is that the vma iterator then detects the misconfigured maple state
> > and resets, thus ensuring the tree is not corrupted - but ultimately
> > causes a failure when there are no nodes left.
> >
> > Detect a misconfigured maple state in the mas_preallocate() code by
> > examining the current location and planned write to ensure a reset is
> > done if required.  The performance impacts are minimal and within the
> > noise in initial testing.
> 
> With this fix applied I can still see the issue using Hailong's
> reproducers, both the userspace one that he shared over the weekend
> and the one posted at
> https://lore.kernel.org/all/1652f7eb-a51b-4fee-8058-c73af63bacd1@oppo.com/

Thanks.

The test patch also has a few issues which would affect the allocations
(such as not setting the rcu flag on the tree).

Besides that, there are a few bugs here.  One is the incorrect
calculation due to the moving vma iterator - which my patch doesn't
quite address as I've come up with another scenario, and test case.  I
suspect the rule of the vma iterator pointing at the 'correct slot'
(documented but not enforced) saved us a lot of issues here.

I'm undecided if it's worth detecting this being false and resetting,
like in this patch.

The other is to do with MA_STATE_PREALLOC, which will stop
preallocations in bulk insert mode.  If it's left set from the first
mas_preallocate() call, then we won't allocate more nodes and simply
return.  That is, the bulk allocation is causing issues with chaining
mas_preallocate() calls.  There is another issue with this flag not
always being set, and so my testing passed... With the way this month
has been going, I expect more fun yet.

I'll send out another version of this RFC with two (or, I guess more..)
patches soon.

Annoyingly, not all of these bugs exist in upstream - and so the vma
test code is less useful than it will be in the future.  I have
recreated the scenario using the test code, but it didn't create the
same issues.

Thanks,
Liam



      reply	other threads:[~2025-04-29  0:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-28 18:40 Liam R. Howlett
2025-04-28 21:08 ` Suren Baghdasaryan
2025-04-29  0:19   ` Liam R. Howlett [this message]

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=2tf7t2tjwsl7oij255uwsyxvmnxg54vsa2rlogkyrevlv2tr5b@c4kcoju752y7 \
    --to=liam.howlett@oracle.com \
    --cc=Steve.Kang@unisoc.com \
    --cc=hailong.liu@oppo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=maple-tree@lists.infradead.org \
    --cc=sidhartha.kumar@oracle.com \
    --cc=surenb@google.com \
    --cc=willy@infradead.org \
    --cc=zhangpeng.00@bytedance.com \
    --cc=zhaoyang.huang@unisoc.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox