Re: [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store()

[Andrew Morton <akpm@linux-foundation.org>]

On Wed,  1 Apr 2026 08:57:02 +0800 Jackie Liu <liu.yun@linux.dev> wrote:

> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> weighted_interleave_auto_store() fetches old_wi_state inside the
> if (!input) block only. This causes two memory leaks:
> 
> 1. When a user writes "false" and the current mode is already manual,
>    the function returns early without freeing the freshly allocated
>    new_wi_state.
> 
> 2. When a user writes "true", old_wi_state stays NULL because the
>    fetch is skipped entirely. The old state is then overwritten by
>    rcu_assign_pointer() but never freed, since the cleanup path is
>    gated on old_wi_state being non-NULL. A user can trigger this
>    repeatedly by writing "1" in a loop.
> 
> Fix both leaks by moving the old_wi_state fetch before the input
> check, making it unconditional. This also allows a unified early
> return for both "true" and "false" when the requested mode matches
> the current mode.
> 
> Cc: stable@vger.kernel.org # v6.16+
> Link: https://sashiko.dev/#/patchset/20260331100740.84906-1-liu.yun@linux.dev
> Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>

Thanks.

> Changes in v2:
> - Move old_wi_state fetch unconditionally before the input check,
>   instead of just adding kfree() to the early return path
> - Also fix an additional memory leak when writing "true" where the
>   previous wi_state was never freed (Sashiko)

Yes, this has changed a lot since v1, so your removal of the three
Reviewed-by:s is appropriate.

 mm/mempolicy.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

--- a/mm/mempolicy.c~mm-mempolicy-fix-memory-leaks-in-weighted_interleave_auto_store
+++ a/mm/mempolicy.c
@@ -3700,18 +3700,19 @@ static ssize_t weighted_interleave_auto_
 		new_wi_state->iw_table[i] = 1;
 
 	mutex_lock(&wi_state_lock);
-	if (!input) {
-		old_wi_state = rcu_dereference_protected(wi_state,
-					lockdep_is_held(&wi_state_lock));
-		if (!old_wi_state)
-			goto update_wi_state;
-		if (input == old_wi_state->mode_auto) {
-			mutex_unlock(&wi_state_lock);
-			return count;
-		}
+	old_wi_state = rcu_dereference_protected(wi_state,
+				lockdep_is_held(&wi_state_lock));
+
+	if (old_wi_state && input == old_wi_state->mode_auto) {
+		mutex_unlock(&wi_state_lock);
+		kfree(new_wi_state);
+		return count;
+	}
 
-		memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
-					       nr_node_ids * sizeof(u8));
+	if (!input) {
+		if (old_wi_state)
+			memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
+						       nr_node_ids * sizeof(u8));
 		goto update_wi_state;
 	}
 
_