[PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Huiwen He]

mas_pop_node() may return NULL when memory allocation fails or when
mas->sheaf is invalid. Several callers of mas_pop_node() did not check
the return value and directly dereferenced the pointer, which could
lead to a NULL pointer dereference and kernel crash.

Fixes: 54a611b60590 ("maple_tree: add mas_pop_node() helper")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
---
 lib/maple_tree.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 39bb779cb311..524e1cfb4439 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -3085,6 +3085,9 @@ static inline void mas_root_expand(struct ma_state *mas, void *entry)
 	int slot = 0;
 
 	node = mas_pop_node(mas);
+	if (unlikely(!node))
+		return;
+
 	pivots = ma_pivots(node, type);
 	slots = ma_slots(node, type);
 	node->parent = ma_parent_ptr(mas_tree_parent(mas));
@@ -3367,6 +3370,9 @@ static inline void mas_new_root(struct ma_state *mas, void *entry)
 	}
 
 	node = mas_pop_node(mas);
+	if (unlikely(!node))
+		return;
+
 	pivots = ma_pivots(node, type);
 	slots = ma_slots(node, type);
 	node->parent = ma_parent_ptr(mas_tree_parent(mas));
@@ -3506,6 +3512,9 @@ static inline void mas_wr_node_store(struct ma_wr_state *wr_mas,
 		newnode = &reuse;
 	}
 
+	if (unlikely(!newnode))
+		return;
+
 	newnode->parent = mas_mn(mas)->parent;
 	dst_pivots = ma_pivots(newnode, wr_mas->type);
 	dst_slots = ma_slots(newnode, wr_mas->type);
-- 
2.43.0

Re: [PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Markus Elfring]

> mas_pop_node() may return NULL when memory allocation fails or when
> mas->sheaf is invalid. Several callers of mas_pop_node() did not check
> the return value and directly dereferenced the pointer, which could
> lead to a NULL pointer dereference and kernel crash.

Will another imperative wording approach become more helpful for an improved
change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n94

Regards,
Markus

Re: [PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Matthew Wilcox]

On Sat, Oct 18, 2025 at 06:38:11PM +0200, Markus Elfring wrote:
> > mas_pop_node() may return NULL when memory allocation fails or when
> > mas->sheaf is invalid. Several callers of mas_pop_node() did not check
> > the return value and directly dereferenced the pointer, which could
> > lead to a NULL pointer dereference and kernel crash.
> 
> Will another imperative wording approach become more helpful for an improved
> change description?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n94

Markus, stop this.  You've been told before.

Re: [PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Huiwen He]

Hi Markus,

Thanks for your suggestion.
The commit description could be improved to better follow the imperative style.
I'll update it in the later version (maybe v2). 

Best regards,
Huiwen He

Re: [PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Matthew Wilcox]

On Sun, Oct 19, 2025 at 07:49:16PM +0800, Huiwen He wrote:
> Hi Markus,
> 
> Thanks for your suggestion.
> The commit description could be improved to better follow the imperative style.
> I'll update it in the later version (maybe v2). 

Do not send a v2 until somebody has a substantive comment.  I suspect
that what you are doing here is wrong, but I lack the understanding to
explain why it is wrong.

Re: [PATCH] maple_tree: Fix potential NULL pointer dereference if mas_pop_node() fails

[Liam R. Howlett]

* Matthew Wilcox <willy@infradead.org> [251019 16:14]:
> On Sun, Oct 19, 2025 at 07:49:16PM +0800, Huiwen He wrote:
...

> 
> Do not send a v2 until somebody has a substantive comment.  I suspect
> that what you are doing here is wrong, but I lack the understanding to
> explain why it is wrong.

Thanks Matthew.

This is not necessary.

At this point we ALWAYS have enough allocations.

I'm guessing you saw the WARN_ON() and return of NULL and assumed we'd
need to check the return in caller.  This WARN_ON() is in place in case
the calculations are incorrect in some corner case (which has never
happened in mainline), so this will add extra instructions for a
significant amount of calls, especially the mas_wr_node_store() path,
with no chance of catching an error.

In fact, the only time I've seen the tree fail to allocate enough memory
is when syzbot fails allocations - and that will happen in the
preallocation stage, which does check the return.

So, thanks for looking but this patch is unnecessary.

Thanks,
Liam