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

[Joshua Hahn]

On Mon, 2 Jun 2025 15:01:08 +0900 Harry Yoo <harry.yoo@xxxxxxxxxx> 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