linux-kernel - Re: mm/mempolicy.c:3719:1-6: ERROR: invalid free of structure field

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250602145240.1868958-1-joshua.hahnjy@gmail.com>
Date: Mon,  2 Jun 2025 07:52:39 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Harry Yoo <harry.yoo@...cle.com>
Cc: kernel test robot <lkp@...el.com>,
	oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Memory Management List <linux-mm@...ck.org>,
	Gregory Price <gourry@...rry.net>,
	Huang Ying <ying.huang@...ux.alibaba.com>,
	Honggyu Kim <honggyu.kim@...com>
Subject: Re: mm/mempolicy.c:3719:1-6: ERROR: invalid free of structure field

On Mon, 2 Jun 2025 15:01:08 +0900 Harry Yoo <harry.yoo@...cle.com> wrote:

> On Sun, Jun 01, 2025 at 01:34:46PM +0800, kernel test robot wrote:
> > cocci warnings: (new ones prefixed by >>)
> > >> mm/mempolicy.c:3719:1-6: ERROR: invalid free of structure field
> > 
> > vim +3719 mm/mempolicy.c
> > 
> >   3700	
> >   3701	static void wi_state_free(void)
> >   3702	{
> >   3703		struct weighted_interleave_state *old_wi_state;
> >   3704	
> >   3705		mutex_lock(&wi_state_lock);
> >   3706	
> >   3707		old_wi_state = rcu_dereference_protected(wi_state,
> >   3708				lockdep_is_held(&wi_state_lock));
> >   3709		if (!old_wi_state) {
> >   3710			mutex_unlock(&wi_state_lock);
> >   3711			goto out;
> >   3712		}
> >   3713	
> >   3714		rcu_assign_pointer(wi_state, NULL);
> >   3715		mutex_unlock(&wi_state_lock);
> >   3716		synchronize_rcu();
> >   3717		kfree(old_wi_state);
> >   3718	out:
> > > 3719		kfree(&wi_group->wi_kobj);
> 
> Hmm maybe Joshua meant kfree(wi_group)?
> 
> Anyway, practically it's the same as kfree(wi_group) and something strange
> is happening there.
> 
> in add_weighted_interleave_group() (the only caller of wi_cleanup()),
> kobject_del() and kobject_put() are called after wi_cleanup() freed
> wi_group in the error path.

Hi Harry,

Thanks for your suggestion and insight! This is totally a slip-up on my end.

I completely missed the kobject_{put, delete} that gets called immediately
after this, which is embarrassing because rebasing on top of Rakie's patch
(which introduces those proper freeing calls) was the main focus of this v8.

>From what I can tell, I think the solution here is to just remove the goto
statement entirely. There is no need to free the wi_group here, and it
would also be bad practice to do more than the function name suggests anyways.

Let me send a patch that gets rid of the goto statement, and just returns if
there is no old_wi_state. While I'm at it, I'll send in a patch from
David Hildenbrand that is an optimization in this area. 

Thanks again for taking a look Harry, hope you have a great day!
Joshua

> 
> >   3720	}
> >   3721	
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> 
> -- 
> Cheers,
> Harry / Hyeonggon