Message-ID: <2tf7t2tjwsl7oij255uwsyxvmnxg54vsa2rlogkyrevlv2tr5b@c4kcoju752y7>
Date: Mon, 28 Apr 2025 20:19:52 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: sidhartha.kumar@...cle.com, maple-tree@...ts.infradead.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Zhaoyang Huang <zhaoyang.huang@...soc.com>,
        Hailong Liu <hailong.liu@...o.com>,
        Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
        "zhangpeng . 00 @ bytedance . com" <zhangpeng.00@...edance.com>,
        Steve Kang <Steve.Kang@...soc.com>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [RFC PATCH v6.6] maple_tree: Fix mas_prealloc() reset

* Suren Baghdasaryan <surenb@...gle.com> [250428 17:08]:
> On Mon, Apr 28, 2025 at 12:02 PM Liam R. Howlett
> <Liam.Howlett@...cle.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

Powered by blists - more mailing lists