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

[PATCH v2] 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 NULL check after the sheaf allocation to bail out early.

Signed-off-by: Josh Law <objecting@objecting.org>
---
v2: drop WARN_ON_ONCE — this is a recoverable allocation failure,
    not a state that warrants a warning splat.

 lib/maple_tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 739918e859e5..1eaaa5f964e9 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 (!ret)
+		return NULL;

 out:
 	memset(ret, 0, sizeof(*ret));
--
2.34.1

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

[Liam R. Howlett]

* Josh Law <hlcj1234567@gmail.com> [260313 03:31]:
> 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 NULL check after the sheaf allocation to bail out early.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
> v2: drop WARN_ON_ONCE — this is a recoverable allocation failure,
>     not a state that warrants a warning splat.
> 
>  lib/maple_tree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 739918e859e5..1eaaa5f964e9 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 (!ret)
> +		return NULL;
> 

This isn't necessary, as explained to v1 after this was sent, I think?

What a mess.  I'm objecting.

Liam