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

[Joshua Hahn <joshua.hahnjy@gmail.com>]

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