[PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

If kmem_cache_alloc_from_sheaf() returns NULL (possible under
GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).

Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
early, matching the existing pattern for the !mas->sheaf case above.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/maple_tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 739918e859e5..87a2ba6468ca 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
 		return NULL;
 
 	ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
+	if (WARN_ON_ONCE(!ret))
+		return NULL;
 
 out:
 	memset(ret, 0, sizeof(*ret));
-- 
2.34.1

[PATCH 2/3] lib/maple_tree: fix always-true condition in mas_erase()

[Josh Law]

The condition (!mas_is_active(mas) || !mas_is_start(mas)) is always
true because ma_active and ma_start are distinct enum values -- a
state can never be both simultaneously. This causes mas_erase() to
unconditionally reset the status to ma_start, discarding a valid
active walk position.

Change || to && so the reset only fires when the state is neither
active nor start, matching the equivalent guard in mas_walk().

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/maple_tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 87a2ba6468ca..9727dcefbf65 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5574,7 +5574,7 @@ void *mas_erase(struct ma_state *mas)
 	unsigned long index = mas->index;
 	MA_WR_STATE(wr_mas, mas, NULL);
 
-	if (!mas_is_active(mas) || !mas_is_start(mas))
+	if (!mas_is_active(mas) && !mas_is_start(mas))
 		mas->status = ma_start;
 
 write_retry:
-- 
2.34.1

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Andrew Morton]

On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:

> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).

This is such a glaring bug that I wonder if we're missing something.

> Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
> early, matching the existing pattern for the !mas->sheaf case above.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/maple_tree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 739918e859e5..87a2ba6468ca 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
>  		return NULL;
>  
>  	ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
> +	if (WARN_ON_ONCE(!ret))
> +		return NULL;

If we're going to do this then we may as well restore !__GFP_NOWARN,
get more relevant information.

But a GFP_NOWAIT allocation attempt can fail relatively easily so
callers must be equipped to handle it - perhaps no need for any
warning.

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

12 Mar 2026 20:45:32 Andrew Morton <akpm@linux-foundation.org>:

> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>
>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
>
> This is such a glaring bug that I wonder if we're missing something.
>
>> Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
>> early, matching the existing pattern for the !mas->sheaf case above.
>>
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>> lib/maple_tree.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>> index 739918e859e5..87a2ba6468ca 100644
>> --- a/lib/maple_tree.c
>> +++ b/lib/maple_tree.c
>> @@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
>>         return NULL;
>>
>>     ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
>> +   if (WARN_ON_ONCE(!ret))
>> +       return NULL;
>
> If we're going to do this then we may as well restore !__GFP_NOWARN,
> get more relevant information.
>
> But a GFP_NOWAIT allocation attempt can fail relatively easily so
> callers must be equipped to handle it - perhaps no need for any
> warning.

Well, fair enough, but WARN_ON is equivalent to a "oops! Something went wrong! We will continue anyway", NOWARN is quite bad for logging that that went wrong, usually it's BUG_ON that causes said kernel panics and that, which is a bit overkill, that's why I didn't add it, and it warns once, then bails, that's why I'm a bit on the iffy side about adding NOWARN, what's your opinion on this, do you think a NOWARN is better then warn on once?


V/R



Josh law

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

12 Mar 2026 20:49:21 Josh Law <hlcj1234567@gmail.com>:

> 12 Mar 2026 20:45:32 Andrew Morton <akpm@linux-foundation.org>:
>
>> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>>
>>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
>>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
>>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
>>
>> This is such a glaring bug that I wonder if we're missing something.
>>
>>> Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
>>> early, matching the existing pattern for the !mas->sheaf case above.
>>>
>>> Signed-off-by: Josh Law <objecting@objecting.org>
>>> ---
>>> lib/maple_tree.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>>> index 739918e859e5..87a2ba6468ca 100644
>>> --- a/lib/maple_tree.c
>>> +++ b/lib/maple_tree.c
>>> @@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
>>>         return NULL;
>>>
>>>     ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
>>> +   if (WARN_ON_ONCE(!ret))
>>> +       return NULL;
>>
>> If we're going to do this then we may as well restore !__GFP_NOWARN,
>> get more relevant information.
>>
>> But a GFP_NOWAIT allocation attempt can fail relatively easily so
>> callers must be equipped to handle it - perhaps no need for any
>> warning.
>
> Well, fair enough, but WARN_ON is equivalent to a "oops! Something went wrong! We will continue anyway", NOWARN is quite bad for logging that that went wrong, usually it's BUG_ON that causes said kernel panics and that, which is a bit overkill, that's why I didn't add it, and it warns once, then bails, that's why I'm a bit on the iffy side about adding NOWARN, what's your opinion on this, do you think a NOWARN is better then warn on once?
>
>
> V/R
>
>
>
> Josh law

I checked the callers as you suggested. In lib/maple_tree.c at lines 2352 and 6039, mas_pop_node() is called inside loops where the return value is used immediately (passed to ma_mnode_ptr or bitwise-ORed) without any NULL validation.
If kmem_cache_alloc_from_sheaf() fails under GFP_NOWAIT pressure, these callers will trigger a kernel panic.

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

12 Mar 2026 20:56:16 Josh Law <hlcj1234567@gmail.com>:

> 12 Mar 2026 20:49:21 Josh Law <hlcj1234567@gmail.com>:
>
>> 12 Mar 2026 20:45:32 Andrew Morton <akpm@linux-foundation.org>:
>>
>>> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>>>
>>>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
>>>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
>>>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
>>>
>>> This is such a glaring bug that I wonder if we're missing something.
>>>
>>>> Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
>>>> early, matching the existing pattern for the !mas->sheaf case above.
>>>>
>>>> Signed-off-by: Josh Law <objecting@objecting.org>
>>>> ---
>>>> lib/maple_tree.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
>>>> index 739918e859e5..87a2ba6468ca 100644
>>>> --- a/lib/maple_tree.c
>>>> +++ b/lib/maple_tree.c
>>>> @@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
>>>>         return NULL;
>>>>
>>>>     ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
>>>> +   if (WARN_ON_ONCE(!ret))
>>>> +       return NULL;
>>>
>>> If we're going to do this then we may as well restore !__GFP_NOWARN,
>>> get more relevant information.
>>>
>>> But a GFP_NOWAIT allocation attempt can fail relatively easily so
>>> callers must be equipped to handle it - perhaps no need for any
>>> warning.
>>
>> Well, fair enough, but WARN_ON is equivalent to a "oops! Something went wrong! We will continue anyway", NOWARN is quite bad for logging that that went wrong, usually it's BUG_ON that causes said kernel panics and that, which is a bit overkill, that's why I didn't add it, and it warns once, then bails, that's why I'm a bit on the iffy side about adding NOWARN, what's your opinion on this, do you think a NOWARN is better then warn on once?
>>
>>
>> V/R
>>
>>
>>
>> Josh law
>
> I checked the callers as you suggested. In lib/maple_tree.c at lines 2352 and 6039, mas_pop_node() is called inside loops where the return value is used immediately (passed to ma_mnode_ptr or bitwise-ORed) without any NULL validation.
> If kmem_cache_alloc_from_sheaf() fails under GFP_NOWAIT pressure, these callers will trigger a kernel panic.

Yeah in my opinion I think this may need to be merged.. if you would like I can add the NOWARN

V/R

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Alice Ryhl]

On Thu, Mar 12, 2026 at 08:49:20PM +0000, Josh Law wrote:
> 12 Mar 2026 20:45:32 Andrew Morton <akpm@linux-foundation.org>:
> 
> > On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> >
> >> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
> >> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
> >> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
> >
> > This is such a glaring bug that I wonder if we're missing something.
> >
> >> Add a WARN_ON_ONCE NULL check after the sheaf allocation to bail out
> >> early, matching the existing pattern for the !mas->sheaf case above.
> >>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >> ---
> >> lib/maple_tree.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> >> index 739918e859e5..87a2ba6468ca 100644
> >> --- a/lib/maple_tree.c
> >> +++ b/lib/maple_tree.c
> >> @@ -1063,6 +1063,8 @@ static __always_inline struct maple_node *mas_pop_node(struct ma_state *mas)
> >>         return NULL;
> >>
> >>     ret = kmem_cache_alloc_from_sheaf(maple_node_cache, GFP_NOWAIT, mas->sheaf);
> >> +   if (WARN_ON_ONCE(!ret))
> >> +       return NULL;
> >
> > If we're going to do this then we may as well restore !__GFP_NOWARN,
> > get more relevant information.
> >
> > But a GFP_NOWAIT allocation attempt can fail relatively easily so
> > callers must be equipped to handle it - perhaps no need for any
> > warning.
> 
> Well, fair enough, but WARN_ON is equivalent to a "oops! Something
> went wrong! We will continue anyway", NOWARN is quite bad for logging
> that that went wrong, usually it's BUG_ON that causes said kernel
> panics and that, which is a bit overkill, that's why I didn't add it,
> and it warns once, then bails, that's why I'm a bit on the iffy side
> about adding NOWARN, what's your opinion on this, do you think a
> NOWARN is better then warn on once?

The WARN_ON option must only be used for conditions that indicate a
kernel bug. Memory pressure is not a kernel bug, so WARN_ON is wrong
here. In fact, depending on kernel configuration, WARN_ON_ONCE may crash
the kernel.

Alice

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Pedro Falcato]

On Thu, Mar 12, 2026 at 01:45:31PM -0700, Andrew Morton wrote:
> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> 
> > If kmem_cache_alloc_from_sheaf() returns NULL (possible under
> > GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
> > and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
> 
> This is such a glaring bug that I wonder if we're missing something.

According to my local copy of lib/maple_tree.c:

mas_pop_node() - Get a previously allocated maple node from the maple state.

Note the "previously" :) kmem_cache_alloc_from_sheaf() can only fail if you
run out of objects in the sheaf.

So yeah, this "bug" looks bogus.

-- 
Pedro

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

12 Mar 2026 23:22:48 Pedro Falcato <pfalcato@suse.de>:

> On Thu, Mar 12, 2026 at 01:45:31PM -0700, Andrew Morton wrote:
>> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>>
>>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
>>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
>>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
>>
>> This is such a glaring bug that I wonder if we're missing something.
>
> According to my local copy of lib/maple_tree.c:
>
> mas_pop_node() - Get a previously allocated maple node from the maple state.
>
> Note the "previously" :) kmem_cache_alloc_from_sheaf() can only fail if you
> run out of objects in the sheaf.
>
> So yeah, this "bug" looks bogus.
>
> --
> Pedro

Hi Pedro,

I see the comment regarding 'previously allocated' nodes. However,
mas_pop_node() explicitly calls kmem_cache_alloc_from_sheaf() with
GFP_NOWAIT. If there is any path—even an unexpected one—where the
sheaf is exhausted or the allocator fails, the code immediately
performs a memset on the NULL pointer.

Even if this is a 'should never happen' scenario, returning NULL is
safer than a kernel panic. As Andrew noted, the current structure
allows a fall-through directly into a dereference. My patch ensures
we handle that edge case safely.

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Pedro Falcato]

On Fri, Mar 13, 2026 at 07:17:17AM +0000, Josh Law wrote:
> 12 Mar 2026 23:22:48 Pedro Falcato <pfalcato@suse.de>:
> 
> > On Thu, Mar 12, 2026 at 01:45:31PM -0700, Andrew Morton wrote:
> >> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
> >>
> >>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
> >>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
> >>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
> >>
> >> This is such a glaring bug that I wonder if we're missing something.
> >
> > According to my local copy of lib/maple_tree.c:
> >
> > mas_pop_node() - Get a previously allocated maple node from the maple state.
> >
> > Note the "previously" :) kmem_cache_alloc_from_sheaf() can only fail if you
> > run out of objects in the sheaf.
> >
> > So yeah, this "bug" looks bogus.
> >
> > --
> > Pedro
> 
> Hi Pedro,
> 
> I see the comment regarding 'previously allocated' nodes. However,
> mas_pop_node() explicitly calls kmem_cache_alloc_from_sheaf() with
> GFP_NOWAIT. If there is any path—even an unexpected one—where the
> sheaf is exhausted or the allocator fails, the code immediately
> performs a memset on the NULL pointer.

And? This does not happen, simply. If it does, your maple tree is hosed
and, really, you're not recovering from it.

> 
> Even if this is a 'should never happen' scenario, returning NULL is
> safer than a kernel panic. As Andrew noted, the current structure
> allows a fall-through directly into a dereference. My patch ensures
> we handle that edge case safely.

... and now because none of the mas_pop_node() callers ever checks for NULL
(why would they, they preallocated those same nodes before), you safely
dereference NULL away from mas_pop_node!.

-- 
Pedro

Re: [PATCH 1/3] lib/maple_tree: fix potential NULL dereference in mas_pop_node()

[Josh Law]

13 Mar 2026 09:05:37 Pedro Falcato <pfalcato@suse.de>:

> On Fri, Mar 13, 2026 at 07:17:17AM +0000, Josh Law wrote:
>> 12 Mar 2026 23:22:48 Pedro Falcato <pfalcato@suse.de>:
>>
>>> On Thu, Mar 12, 2026 at 01:45:31PM -0700, Andrew Morton wrote:
>>>> On Thu, 12 Mar 2026 18:40:53 +0000 Josh Law <hlcj1234567@gmail.com> wrote:
>>>>
>>>>> If kmem_cache_alloc_from_sheaf() returns NULL (possible under
>>>>> GFP_NOWAIT pressure), mas_pop_node() falls through to the out label
>>>>> and dereferences the NULL pointer in memset(ret, 0, sizeof(*ret)).
>>>>
>>>> This is such a glaring bug that I wonder if we're missing something.
>>>
>>> According to my local copy of lib/maple_tree.c:
>>>
>>> mas_pop_node() - Get a previously allocated maple node from the maple state.
>>>
>>> Note the "previously" :) kmem_cache_alloc_from_sheaf() can only fail if you
>>> run out of objects in the sheaf.
>>>
>>> So yeah, this "bug" looks bogus.
>>>
>>> --
>>> Pedro
>>
>> Hi Pedro,
>>
>> I see the comment regarding 'previously allocated' nodes. However,
>> mas_pop_node() explicitly calls kmem_cache_alloc_from_sheaf() with
>> GFP_NOWAIT. If there is any path—even an unexpected one—where the
>> sheaf is exhausted or the allocator fails, the code immediately
>> performs a memset on the NULL pointer.
>
> And? This does not happen, simply. If it does, your maple tree is hosed
> and, really, you're not recovering from it.
>
>>
>> Even if this is a 'should never happen' scenario, returning NULL is
>> safer than a kernel panic. As Andrew noted, the current structure
>> allows a fall-through directly into a dereference. My patch ensures
>> we handle that edge case safely.
>
> ... and now because none of the mas_pop_node() callers ever checks for NULL
> (why would they, they preallocated those same nodes before), you safely
> dereference NULL away from mas_pop_node!.
>
> --
> Pedro

Hi Pedro,

I see your point regarding the invariants of the sheaf. If the
pre-allocation logic is guaranteed, then a NULL return here implies
a fundamental corruption of the maple state.

My concern was primarily the 'fall-through' structure which makes the
dereference look accidental rather than intentional. However, if
the callers aren't prepared to handle a NULL return anyway, simply
returning NULL doesn't solve the underlying panic.

I'll take this as a lesson in understanding function invariants
before jumping to defensive checks. Thanks for the technical
explanation.


V/R

Josh law