Re: [RESEND PATCH v4 10/15] fs/resctrl: Reset the kernel-mode binding when an rdtgroup is removed

[Babu Moger <babu.moger@amd.com>]

On 7/7/26 16:50, Babu Moger wrote:
> Resctrl keeps track of the rdtgroup backing the active global kernel-mode
> policy, including the group's CPU mask used to program kernel-mode
> associations.
> 
> When that rdtgroup is deleted, resctrl must update both its internal state
> and the hardware configuration associated with the group's kmode_cpu_mask.
> 
> Introduce rdtgroup_config_kmode_reset() to disable the active kernel-mode
> association for the bound group's CPU mask and to clear its kernel-mode
> state. Invoke this helper from rdtgroup_kmode_detach(), and reset
> resctrl_kcfg to INHERIT_CTRL_AND_MON. Ensure rdtgroup_kmode_detach() is
> called during group removal and filesystem teardown, before the bound
> rdtgroup is freed.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v4: Re-wrote the changelog.
>      Added the call free_all_child_rdtgrp() and rmdir_all_sub()
>      Simplified the code comments.
> 
> v3: New patch to handle the kernel_mode clean up.
> ---
>   fs/resctrl/rdtgroup.c | 62 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 0d5c94169d03..21659fd75850 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -1127,6 +1127,61 @@ static int resctrl_kernel_mode_show(struct kernfs_open_file *of,
>   	return ret;
>   }
>   
> +/**
> + * rdtgroup_config_kmode_reset() - Tear down the kernel-mode binding on @rdtgrp
> + * @rdtgrp:	Resctrl group whose kernel-mode binding is being released.
> + *		May be %NULL when no group is currently bound, in which case
> + *		this is a no-op.
> + * @kmode:	Kernel-mode policy currently active on @rdtgrp.
> + *
> + * Reset the kernel-mode binding on the CPUs in @rdtgrp's @kmode_cpu_mask.
> + */
> +static void rdtgroup_config_kmode_reset(struct rdtgroup *rdtgrp,
> +					enum resctrl_kernel_mode kmode)
> +{
> +	bool assign_mon = false;
> +
> +	if (!rdtgrp)
> +		return;
> +
> +	if (kmode == INHERIT_CTRL_AND_MON)
> +		goto out_clear;
> +
> +	if (kmode == GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU)
> +		assign_mon = true;
> +
> +	resctrl_arch_configure_kmode(&rdtgrp->kmode_cpu_mask, rdtgrp->closid,
> +				     rdtgrp->mon.rmid, assign_mon, false);
> +
> +out_clear:
> +	cpumask_clear(&rdtgrp->kmode_cpu_mask);
> +	rdtgrp->kmode = false;
> +}
> +
> +/**
> + * rdtgroup_kmode_detach() - Detach @rdtgrp from kernel-mode assignment
> + * @rdtgrp: Resctrl group being removed or torn down
> + *
> + * If @rdtgrp is bound to the active kernel-mode assignment, disable the
> + * hardware association programmed for that group and reset the kernel-mode
> + * to INHERIT_CTRL_AND_MON.
> + */
> +static void rdtgroup_kmode_detach(struct rdtgroup *rdtgrp)
> +{
> +	if (!rdtgrp || !rdtgrp->kmode)
> +		return;
> +
> +	if (resctrl_kcfg.k_rdtgrp != rdtgrp) {
> +		pr_warn("resctrl: kernel-mode group not valid\n");
> +		return;
> +	}
> +
> +	rdtgroup_config_kmode_reset(rdtgrp, resctrl_kcfg.kmode_cur);
> +
> +	resctrl_kcfg.k_rdtgrp = NULL;
> +	resctrl_kcfg.kmode_cur = INHERIT_CTRL_AND_MON;
> +}
> +
>   void *rdt_kn_parent_priv(struct kernfs_node *kn)
>   {
>   	/*
> @@ -3215,6 +3270,7 @@ static void free_all_child_rdtgrp(struct rdtgroup *rdtgrp)
>   
>   	head = &rdtgrp->mon.crdtgrp_list;
>   	list_for_each_entry_safe(sentry, stmp, head, mon.crdtgrp_list) {
> +		rdtgroup_kmode_detach(sentry);
>   		rdtgroup_unassign_cntrs(sentry);
>   		free_rmid(sentry->closid, sentry->mon.rmid);
>   		list_del(&sentry->mon.crdtgrp_list);
> @@ -3252,6 +3308,7 @@ static void rmdir_all_sub(void)
>   		cpumask_or(&rdtgroup_default.cpu_mask,
>   			   &rdtgroup_default.cpu_mask, &rdtgrp->cpu_mask);
>   
> +		rdtgroup_kmode_detach(rdtgrp);
>   		rdtgroup_unassign_cntrs(rdtgrp);
>   
>   		if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP ||
> @@ -3347,6 +3404,7 @@ static void resctrl_fs_teardown(void)
>   		return;
>   
>   	rmdir_all_sub();
> +	rdtgroup_kmode_detach(&rdtgroup_default);
>   	rdtgroup_unassign_cntrs(&rdtgroup_default);
>   	mon_put_kn_priv();
>   	rdt_pseudo_lock_release();
> @@ -4149,6 +4207,8 @@ static int rdtgroup_rmdir_mon(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask)
>   	u32 closid, rmid;
>   	int cpu;
>   
> +	rdtgroup_kmode_detach(rdtgrp);
> +
>   	/* Give any tasks back to the parent group */
>   	rdt_move_group_tasks(rdtgrp, prdtgrp, tmpmask);
>   
> @@ -4199,6 +4259,8 @@ static int rdtgroup_rmdir_ctrl(struct rdtgroup *rdtgrp, cpumask_var_t tmpmask)
>   	u32 closid, rmid;
>   	int cpu;
>   
> +	rdtgroup_kmode_detach(rdtgrp);
> +
>   	/* Give any tasks back to the default group */
>   	rdt_move_group_tasks(rdtgrp, &rdtgroup_default, tmpmask);
>   

Sashiko reported:

https://sashiko.dev/#/patchset/cover.1783461016.git.babu.moger%40amd.com?part=10


--- Patch [10]: [RESEND PATCH v4 10/15] fs/resctrl: Reset the 
kernel-mode binding when an rdtgroup is removed ---
commit 42354ae9f22a1b788ec34201f40310a30c5f5247
Author: Babu Moger <babu.moger@amd.com>
fs/resctrl: Reset the kernel-mode binding when an rdtgroup is removed
This commit introduces rdtgroup_config_kmode_reset() and
rdtgroup_kmode_detach() to tear down the kernel-mode binding on a resctrl
group. It invokes these helpers during group removal and filesystem teardown
to disable active kernel-mode hardware association and clear internal state.
 > diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
 > index 0d5c94169d03f..21659fd75850e 100644
 > --- a/fs/resctrl/rdtgroup.c
 > +++ b/fs/resctrl/rdtgroup.c
[ ... ]
 > +static void rdtgroup_config_kmode_reset(struct rdtgroup *rdtgrp,
 > +					enum resctrl_kernel_mode kmode)
 > +{
 > +	bool assign_mon = false;
 > +
 > +	if (!rdtgrp)
 > +		return;
 > +
 > +	if (kmode == INHERIT_CTRL_AND_MON)
 > +		goto out_clear;
 > +
 > +	if (kmode == GLOBAL_ASSIGN_CTRL_ASSIGN_MON_PER_CPU)
 > +		assign_mon = true;
 > +
 > +	resctrl_arch_configure_kmode(&rdtgrp->kmode_cpu_mask, rdtgrp->closid,
 > +				     rdtgrp->mon.rmid, assign_mon, false);
Does this teardown properly handle offline CPUs?
If a CPU in rdtgrp->kmode_cpu_mask is offlined before this reset is called,
the underlying IPI might skip the offline CPU, leaving its MSR unmodified.
When the CPU is brought back online, clear_closid_rmid() clears
MSR_IA32_PQR_ASSOC, but does it also clear the newly introduced
MSR_IA32_PQR_PLZA_ASSOC?
If not, the CPU could resume executing with stale Privilege Level Zero
Association state, potentially causing incorrect resource allocation for
kernel mode code.

----------------------------------------------------------------------

Yes. This is an issue.
Need to clear the kmode_cpu_list if the cpu being offlined is part of 
the current kmode. Also, need to disable PLZA on the CPU that is offlined.

Will take care of this in resctrl_offline_cpu(). The changes will go in 
patch 11 (Patch 11: [RESEND PATCH v4 11/15] fs/resctrl: Program 
kernel-mode binding when CPU comes online)



 > +
 > +out_clear:
 > +	cpumask_clear(&rdtgrp->kmode_cpu_mask);
 > +	rdtgrp->kmode = false;
 > +}
[ ... ]
 > @@ -3215,6 +3270,7 @@ static void free_all_child_rdtgrp(struct 
rdtgroup *rdtgrp)
 >
 >  	head = &rdtgrp->mon.crdtgrp_list;
 >  	list_for_each_entry_safe(sentry, stmp, head, mon.crdtgrp_list) {
 > +		rdtgroup_kmode_detach(sentry);
 >  		rdtgroup_unassign_cntrs(sentry);
 >  		free_rmid(sentry->closid, sentry->mon.rmid);
 >  		list_del(&sentry->mon.crdtgrp_list);

This is a pre-existing issue, but can unmounting the filesystem lead to 
a use-after-free here if child monitor groups are present?

If the admin unmounts the resctrl filesystem while a user process holds 
an open file descriptor to a file within a monitor group directory,
resctrl_fs_teardown() calls rmdir_all_sub() which calls
free_all_child_rdtgrp().

A few lines below this hunk, this function frees the monitor group 
(sentry) via rdtgroup_remove() without first calling 
kernfs_remove(sentry->kn).
The sentry->kn node retains a refcount and remains active in the kernfs 
tree. Could a concurrent read() or write() by the user process access 
the freed sentry structure via kn->priv before the tree is finally 
destroyed?
----------------------------------------------------------------------

If a user process still holds the file descriptor, the waitcount is 
non-zero, and rdtgroup_remove(sentry) is not called. As a result, the 
sentry structure is not freed.

Is this actually an issue?

Thanks
Babu