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

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

[Jackie Liu]

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>
---
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)

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

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index cf92bd6a8226..ebe4bc8220b1 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -3706,18 +3706,19 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
 		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));
 
-		memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
-					       nr_node_ids * sizeof(u8));
+	if (old_wi_state && input == old_wi_state->mode_auto) {
+		mutex_unlock(&wi_state_lock);
+		kfree(new_wi_state);
+		return count;
+	}
+
+	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;
 	}
 
-- 
2.51.1

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

[Andrew Morton]

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;
 	}
 
_

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

[Joshua Hahn]

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.

Hi Jackie,

Thank you for the quick turnaround on the patch, and also thank you for fixing
the second bug as well. This looks good to me! So much cleaner than before
as well : -)

Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>

> 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>
> ---
> 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)
> 
>  mm/mempolicy.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index cf92bd6a8226..ebe4bc8220b1 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3706,18 +3706,19 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
>  		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));
>  
> -		memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
> -					       nr_node_ids * sizeof(u8));
> +	if (old_wi_state && input == old_wi_state->mode_auto) {
> +		mutex_unlock(&wi_state_lock);
> +		kfree(new_wi_state);
> +		return count;
> +	}
> +
> +	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;
>  	}
>  
> -- 
> 2.51.1

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

[Donet Tom]

[-- Attachment #1: Type: text/plain, Size: 2752 bytes --]


On 4/1/26 6:27 AM, Jackie Liu 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>
> ---
> 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)
>
>   mm/mempolicy.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index cf92bd6a8226..ebe4bc8220b1 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3706,18 +3706,19 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
>   		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));
>   
> -		memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
> -					       nr_node_ids * sizeof(u8));
> +	if (old_wi_state && input == old_wi_state->mode_auto) {
> +		mutex_unlock(&wi_state_lock);
> +		kfree(new_wi_state);
> +		return count;
> +	}
> +
> +	if (!input) {
> +		if (old_wi_state)
> +			memcpy(new_wi_state->iw_table, old_wi_state->iw_table,
> +						       nr_node_ids * sizeof(u8));

Yes, these are valid issues. This looks good to me.

Reviewed by: Donet Tom <donettom@linux.ibm.com>



>   		goto update_wi_state;
>   	}
>   

[-- Attachment #2: Type: text/html, Size: 4262 bytes --]