* Re: [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store()
2026-04-01 0:57 [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store() Jackie Liu
@ 2026-04-01 3:43 ` Andrew Morton
2026-04-01 14:56 ` Joshua Hahn
2026-04-02 9:04 ` Donet Tom
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-04-01 3:43 UTC (permalink / raw)
To: Jackie Liu
Cc: joshua.hahnjy, gourry, linux-mm, Joshua Hahn, Donet Tom,
Gregory Price, Alistair Popple, Byungchul Park, David Hildenbrand
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;
}
_
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store()
2026-04-01 0:57 [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store() Jackie Liu
2026-04-01 3:43 ` Andrew Morton
@ 2026-04-01 14:56 ` Joshua Hahn
2026-04-02 9:04 ` Donet Tom
2 siblings, 0 replies; 4+ messages in thread
From: Joshua Hahn @ 2026-04-01 14:56 UTC (permalink / raw)
To: Jackie Liu; +Cc: joshua.hahnjy, akpm, gourry, linux-mm
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store()
2026-04-01 0:57 [PATCH v2] mm/mempolicy: fix memory leaks in weighted_interleave_auto_store() Jackie Liu
2026-04-01 3:43 ` Andrew Morton
2026-04-01 14:56 ` Joshua Hahn
@ 2026-04-02 9:04 ` Donet Tom
2 siblings, 0 replies; 4+ messages in thread
From: Donet Tom @ 2026-04-02 9:04 UTC (permalink / raw)
To: Jackie Liu, joshua.hahnjy; +Cc: akpm, gourry, linux-mm
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread