public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
* [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
@ 2026-03-31 10:07 Jackie Liu
  2026-03-31 14:31 ` Joshua Hahn
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jackie Liu @ 2026-03-31 10:07 UTC (permalink / raw)
  To: akpm, joshua.hahnjy; +Cc: linux-mm

From: Jackie Liu <liuyun01@kylinos.cn>

Add the missing kfree(new_wi_state) when the auto mode is already set
to the requested value. When a user writes "false" to the auto sysfs
interface and the current mode is already manual (mode_auto == false),
the function returns early without freeing new_wi_state allocated at
the beginning of the function. This can be triggered repeatedly from
userspace, leaking memory on each write.

Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 mm/mempolicy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index cf92bd6a8226..9ac74178075b 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
 			goto update_wi_state;
 		if (input == old_wi_state->mode_auto) {
 			mutex_unlock(&wi_state_lock);
+			kfree(new_wi_state);
 			return count;
 		}
 
-- 
2.51.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 10:07 [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store() Jackie Liu
@ 2026-03-31 14:31 ` Joshua Hahn
  2026-03-31 16:41 ` Donet Tom
  2026-03-31 16:53 ` Gregory Price
  2 siblings, 0 replies; 8+ messages in thread
From: Joshua Hahn @ 2026-03-31 14:31 UTC (permalink / raw)
  To: Jackie Liu; +Cc: akpm, joshua.hahnjy, linux-mm

On Tue, 31 Mar 2026 18:07:40 +0800 Jackie Liu <liu.yun@linux.dev> wrote:

> From: Jackie Liu <liuyun01@kylinos.cn>

Hi Jackie,

Thank you for the patch! Wow!! This is a great catch. As you note, the
trigger for this memory leak doesn't seem too difficult either.

Everything looks good, thank you! Have a great day : -)

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

> Add the missing kfree(new_wi_state) when the auto mode is already set
> to the requested value. When a user writes "false" to the auto sysfs
> interface and the current mode is already manual (mode_auto == false),
> the function returns early without freeing new_wi_state allocated at
> the beginning of the function. This can be triggered repeatedly from
> userspace, leaking memory on each write.
> 
> Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  mm/mempolicy.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index cf92bd6a8226..9ac74178075b 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
>  			goto update_wi_state;
>  		if (input == old_wi_state->mode_auto) {
>  			mutex_unlock(&wi_state_lock);
> +			kfree(new_wi_state);
>  			return count;
>  		}
>  
> -- 
> 2.51.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 10:07 [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store() Jackie Liu
  2026-03-31 14:31 ` Joshua Hahn
@ 2026-03-31 16:41 ` Donet Tom
  2026-03-31 16:53 ` Gregory Price
  2 siblings, 0 replies; 8+ messages in thread
From: Donet Tom @ 2026-03-31 16:41 UTC (permalink / raw)
  To: Jackie Liu, akpm, joshua.hahnjy; +Cc: linux-mm


Hi Jackie

On 3/31/26 3:37 PM, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> Add the missing kfree(new_wi_state) when the auto mode is already set
> to the requested value. When a user writes "false" to the auto sysfs
> interface and the current mode is already manual (mode_auto == false),
> the function returns early without freeing new_wi_state allocated at
> the beginning of the function. This can be triggered repeatedly from
> userspace, leaking memory on each write.
>
> Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>   mm/mempolicy.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index cf92bd6a8226..9ac74178075b 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
>   			goto update_wi_state;
>   		if (input == old_wi_state->mode_auto) {
>   			mutex_unlock(&wi_state_lock);
> +			kfree(new_wi_state);

Good catch—this looks good to me

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

>   			return count;
>   		}
>   


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 10:07 [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store() Jackie Liu
  2026-03-31 14:31 ` Joshua Hahn
  2026-03-31 16:41 ` Donet Tom
@ 2026-03-31 16:53 ` Gregory Price
  2026-03-31 19:01   ` Andrew Morton
  2 siblings, 1 reply; 8+ messages in thread
From: Gregory Price @ 2026-03-31 16:53 UTC (permalink / raw)
  To: Jackie Liu; +Cc: akpm, joshua.hahnjy, linux-mm

On Tue, Mar 31, 2026 at 06:07:40PM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> Add the missing kfree(new_wi_state) when the auto mode is already set
> to the requested value. When a user writes "false" to the auto sysfs
> interface and the current mode is already manual (mode_auto == false),
> the function returns early without freeing new_wi_state allocated at
> the beginning of the function. This can be triggered repeatedly from
> userspace, leaking memory on each write.
> 
> Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>

Reviewed-by: Gregory Price <gourry@gourry.net>

> ---
>  mm/mempolicy.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index cf92bd6a8226..9ac74178075b 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
>  			goto update_wi_state;
>  		if (input == old_wi_state->mode_auto) {
>  			mutex_unlock(&wi_state_lock);
> +			kfree(new_wi_state);
>  			return count;
>  		}
>  
> -- 
> 2.51.1
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 16:53 ` Gregory Price
@ 2026-03-31 19:01   ` Andrew Morton
  2026-03-31 19:21     ` Joshua Hahn
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2026-03-31 19:01 UTC (permalink / raw)
  To: Gregory Price; +Cc: Jackie Liu, joshua.hahnjy, linux-mm

On Tue, 31 Mar 2026 12:53:40 -0400 Gregory Price <gourry@gourry.net> wrote:

> On Tue, Mar 31, 2026 at 06:07:40PM +0800, Jackie Liu wrote:
> > From: Jackie Liu <liuyun01@kylinos.cn>
> > 
> > Add the missing kfree(new_wi_state) when the auto mode is already set
> > to the requested value. When a user writes "false" to the auto sysfs
> > interface and the current mode is already manual (mode_auto == false),
> > the function returns early without freeing new_wi_state allocated at
> > the beginning of the function. This can be triggered repeatedly from
> > userspace, leaking memory on each write.
> > 
> > Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> 
> ..
>
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
> >  			goto update_wi_state;
> >  		if (input == old_wi_state->mode_auto) {
> >  			mutex_unlock(&wi_state_lock);
> > +			kfree(new_wi_state);
> >  			return count;
> >  		}
> >  

Thanks all.

Am I correct in believing that triggering this leak requires elevated
privileges?

I'll add cc:stable to this and shall queue it for 7.1-rc1. 
This means (I assume) that its entry into the -stable trees might be a
little later than if we were to upstream it immediately.

AI review liked this patch but claims to have found another one:
https://sashiko.dev/#/patchset/20260331100740.84906-1-liu.yun@linux.dev


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 19:01   ` Andrew Morton
@ 2026-03-31 19:21     ` Joshua Hahn
  2026-03-31 19:24       ` Joshua Hahn
  2026-04-01  0:59       ` Jackie Liu
  0 siblings, 2 replies; 8+ messages in thread
From: Joshua Hahn @ 2026-03-31 19:21 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Gregory Price, Jackie Liu, joshua.hahnjy, linux-mm

On Tue, 31 Mar 2026 12:01:10 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 31 Mar 2026 12:53:40 -0400 Gregory Price <gourry@gourry.net> wrote:
> 
> > On Tue, Mar 31, 2026 at 06:07:40PM +0800, Jackie Liu wrote:
> > > From: Jackie Liu <liuyun01@kylinos.cn>
> > > 
> > > Add the missing kfree(new_wi_state) when the auto mode is already set
> > > to the requested value. When a user writes "false" to the auto sysfs
> > > interface and the current mode is already manual (mode_auto == false),
> > > the function returns early without freeing new_wi_state allocated at
> > > the beginning of the function. This can be triggered repeatedly from
> > > userspace, leaking memory on each write.
> > > 
> > > Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> > > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > 
> > ..
> >
> > > --- a/mm/mempolicy.c
> > > +++ b/mm/mempolicy.c
> > > @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
> > >  			goto update_wi_state;
> > >  		if (input == old_wi_state->mode_auto) {
> > >  			mutex_unlock(&wi_state_lock);
> > > +			kfree(new_wi_state);
> > >  			return count;
> > >  		}
> > >  
> 
> Thanks all.
> 
> Am I correct in believing that triggering this leak requires elevated
> privileges?

Hello Andrew, I hope you are doing well : -)

Yes indeed, writing to the file requires elevated privileges.

While going to check this out, however, I noticed a different bug which is
that the file is no longer called "auto", but called "__auto_type".
I suspected this was the result of a newly defined macro, and surely...
commit 6cce897a37dc "compiler_types.h: add "auto" as a macro for "__auto_type""
seems to have defined auto to always expand out to __auto_type.

Of course for using the __ATTR(name, permissions, show, store) macro this
is bad because writing "auto" there no longer works...

I'll send up a quick fix to just manually write out the name instead.
Doing a quick grep for the pattern thankfully seems to only point to this.
I do think it is a bit weird to pass a raw, unquoted string into the macro...

> I'll add cc:stable to this and shall queue it for 7.1-rc1. 
> This means (I assume) that its entry into the -stable trees might be a
> little later than if we were to upstream it immediately.
> 
> AI review liked this patch but claims to have found another one:
> https://sashiko.dev/#/patchset/20260331100740.84906-1-liu.yun@linux.dev

Sashiko seems to be correct here. Pretty neat that it was able to catch
a related bug when analyzing the correctness of this fix!

I can send up a fix for this one too, or leave it to you Jackie, whichever
you prefer! Just let me know : -) I'll go ahead and send a fix for the
naming issue though, since that one seems orthogonal to this.

Thank you, and I hope you have a great day!
Joshua



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 19:21     ` Joshua Hahn
@ 2026-03-31 19:24       ` Joshua Hahn
  2026-04-01  0:59       ` Jackie Liu
  1 sibling, 0 replies; 8+ messages in thread
From: Joshua Hahn @ 2026-03-31 19:24 UTC (permalink / raw)
  To: Joshua Hahn; +Cc: Andrew Morton, Gregory Price, Jackie Liu, linux-mm

On Tue, 31 Mar 2026 12:21:04 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> On Tue, 31 Mar 2026 12:01:10 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 31 Mar 2026 12:53:40 -0400 Gregory Price <gourry@gourry.net> wrote:
> > 
> > > On Tue, Mar 31, 2026 at 06:07:40PM +0800, Jackie Liu wrote:
> > > > From: Jackie Liu <liuyun01@kylinos.cn>
> > > > 
> > > > Add the missing kfree(new_wi_state) when the auto mode is already set
> > > > to the requested value. When a user writes "false" to the auto sysfs
> > > > interface and the current mode is already manual (mode_auto == false),
> > > > the function returns early without freeing new_wi_state allocated at
> > > > the beginning of the function. This can be triggered repeatedly from
> > > > userspace, leaking memory on each write.
> > > > 
> > > > Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> > > > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> > > 
> > > ..
> > >
> > > > --- a/mm/mempolicy.c
> > > > +++ b/mm/mempolicy.c
> > > > @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
> > > >  			goto update_wi_state;
> > > >  		if (input == old_wi_state->mode_auto) {
> > > >  			mutex_unlock(&wi_state_lock);
> > > > +			kfree(new_wi_state);
> > > >  			return count;
> > > >  		}
> > > >  
> > 
> > Thanks all.
> > 
> > Am I correct in believing that triggering this leak requires elevated
> > privileges?
> 
> Hello Andrew, I hope you are doing well : -)
> 
> Yes indeed, writing to the file requires elevated privileges.
> 
> While going to check this out, however, I noticed a different bug which is
> that the file is no longer called "auto", but called "__auto_type".
> I suspected this was the result of a newly defined macro, and surely...
> commit 6cce897a37dc "compiler_types.h: add "auto" as a macro for "__auto_type""
> seems to have defined auto to always expand out to __auto_type.
> 
> Of course for using the __ATTR(name, permissions, show, store) macro this
> is bad because writing "auto" there no longer works...
> 
> I'll send up a quick fix to just manually write out the name instead.
> Doing a quick grep for the pattern thankfully seems to only point to this.
> I do think it is a bit weird to pass a raw, unquoted string into the macro...

(I just want to clarify that I am not suggesting that 6cce897a37dc was
incorrect. In hindsight, I should have been more wary about using a name
like "auto" when it is already a C keyword)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
  2026-03-31 19:21     ` Joshua Hahn
  2026-03-31 19:24       ` Joshua Hahn
@ 2026-04-01  0:59       ` Jackie Liu
  1 sibling, 0 replies; 8+ messages in thread
From: Jackie Liu @ 2026-04-01  0:59 UTC (permalink / raw)
  To: Joshua Hahn, Andrew Morton; +Cc: Gregory Price, joshua.hahnjy, linux-mm

2026年4月1日 03:21, "Joshua Hahn" <joshua.hahnjy@gmail.com mailto:joshua.hahnjy@gmail.com?to=%22Joshua%20Hahn%22%20%3Cjoshua.hahnjy%40gmail.com%3E > 写到:


> 
> On Tue, 31 Mar 2026 12:01:10 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > 
> > On Tue, 31 Mar 2026 12:53:40 -0400 Gregory Price <gourry@gourry.net> wrote:
> >  
> >  On Tue, Mar 31, 2026 at 06:07:40PM +0800, Jackie Liu wrote:
> >  > From: Jackie Liu <liuyun01@kylinos.cn>
> >  > 
> >  > Add the missing kfree(new_wi_state) when the auto mode is already set
> >  > to the requested value. When a user writes "false" to the auto sysfs
> >  > interface and the current mode is already manual (mode_auto == false),
> >  > the function returns early without freeing new_wi_state allocated at
> >  > the beginning of the function. This can be triggered repeatedly from
> >  > userspace, leaking memory on each write.
> >  > 
> >  > Fixes: e341f9c3c841 ("mm/mempolicy: Weighted Interleave Auto-tuning")
> >  > Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >  
> >  ..
> > 
> >  > --- a/mm/mempolicy.c
> >  > +++ b/mm/mempolicy.c
> >  > @@ -3713,6 +3713,7 @@ static ssize_t weighted_interleave_auto_store(struct kobject *kobj,
> >  > goto update_wi_state;
> >  > if (input == old_wi_state->mode_auto) {
> >  > mutex_unlock(&wi_state_lock);
> >  > + kfree(new_wi_state);
> >  > return count;
> >  > }
> >  > 
> >  
> >  Thanks all.
> >  
> >  Am I correct in believing that triggering this leak requires elevated
> >  privileges?
> > 
> Hello Andrew, I hope you are doing well : -)
> 
> Yes indeed, writing to the file requires elevated privileges.
> 
> While going to check this out, however, I noticed a different bug which is
> that the file is no longer called "auto", but called "__auto_type".
> I suspected this was the result of a newly defined macro, and surely...
> commit 6cce897a37dc "compiler_types.h: add "auto" as a macro for "__auto_type""
> seems to have defined auto to always expand out to __auto_type.
> 
> Of course for using the __ATTR(name, permissions, show, store) macro this
> is bad because writing "auto" there no longer works...
> 
> I'll send up a quick fix to just manually write out the name instead.
> Doing a quick grep for the pattern thankfully seems to only point to this.
> I do think it is a bit weird to pass a raw, unquoted string into the macro...
> 
> > 
> > I'll add cc:stable to this and shall queue it for 7.1-rc1. 
> >  This means (I assume) that its entry into the -stable trees might be a
> >  little later than if we were to upstream it immediately.
> >  
> >  AI review liked this patch but claims to have found another one:
> >  https://sashiko.dev/#/patchset/20260331100740.84906-1-liu.yun@linux.dev
> > 
> Sashiko seems to be correct here. Pretty neat that it was able to catch
> a related bug when analyzing the correctness of this fix!

Thank you. I quickly wrote a patch and sent it. Sashiko looks amazing.

The link is https://lore.kernel.org/all/20260401005702.7096-1-liu.yun@linux.dev/

Please help me review it again.

-- 
Jackie Liu

> 
> I can send up a fix for this one too, or leave it to you Jackie, whichever
> you prefer! Just let me know : -) I'll go ahead and send a fix for the
> naming issue though, since that one seems orthogonal to this.
> 
> Thank you, and I hope you have a great day!
> Joshua
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-04-01  0:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 10:07 [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store() Jackie Liu
2026-03-31 14:31 ` Joshua Hahn
2026-03-31 16:41 ` Donet Tom
2026-03-31 16:53 ` Gregory Price
2026-03-31 19:01   ` Andrew Morton
2026-03-31 19:21     ` Joshua Hahn
2026-03-31 19:24       ` Joshua Hahn
2026-04-01  0:59       ` Jackie Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox