linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Joshua Hahn <joshua.hahnjy@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Gregory Price <gourry@gourry.net>, Jackie Liu <liu.yun@linux.dev>,
	joshua.hahnjy@gmail.com, linux-mm@kvack.org
Subject: Re: [PATCH] mm/mempolicy: fix memory leak in weighted_interleave_auto_store()
Date: Tue, 31 Mar 2026 12:21:04 -0700	[thread overview]
Message-ID: <20260331192105.109847-1-joshua.hahnjy@gmail.com> (raw)
In-Reply-To: <20260331120110.9136f80efa9c41146f1b92a2@linux-foundation.org>

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



  reply	other threads:[~2026-03-31 19:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 10:07 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 [this message]
2026-03-31 19:24       ` Joshua Hahn
2026-04-01  0:59       ` Jackie Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260331192105.109847-1-joshua.hahnjy@gmail.com \
    --to=joshua.hahnjy@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=gourry@gourry.net \
    --cc=linux-mm@kvack.org \
    --cc=liu.yun@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox