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

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

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   7d4e49a77d9930c69751b9192448fda6ff9100f1
commit: e341f9c3c8412e57fe0042a33a2640245ecdf619 mm/mempolicy: Weighted Interleave Auto-tuning
date:   11 days ago
config: loongarch-randconfig-r054-20250601 (https://download.01.org/0day-ci/archive/20250601/202506011545.Fduxqxqj-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506011545.Fduxqxqj-lkp@intel.com/

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);
  3720	}
  3721	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Harry Yoo]

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.

>   3720	}
>   3721	
> 
> -- 
> 0-DAY CI Kernel Test Service

-- 
Cheers,
Harry / Hyeonggon

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@oracle.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