Re: [PATCH v3 5/9] fs/resctrl: Prevent use-after-free in rdtgroup_kn_put()

[Ben Horgan]

Hi Reinette,

On 5/22/26 20:15, Reinette Chatre wrote:
> A struct rdtgroup is reference counted via rdtgroup::waitcount. Callers
> that need the structure to remain valid across a sleep (while waiting on
> acquiring rdtgroup_mutex) take a reference with rdtgroup_kn_get() and
> release it with rdtgroup_kn_put(). The release path is intended to serve
> as the fallback freer: if the count drops to zero and the group has
> already been marked RDT_DELETED, rdtgroup_kn_put() frees the structure.
> The bulk teardown paths free_all_child_rdtgrp() and rmdir_all_sub()
> resulting from a resctrl directory remove or resctrl fs unmount act as
> the primary freer: they hold rdtgroup_mutex and free each rdtgroup whose
> waitcount is zero, otherwise they set RDT_DELETED and leave the freeing
> to the last waiter.
> 
> These two freers race. rdtgroup_kn_put() commits waitcount == 0 with
> atomic_dec_and_test() outside rdtgroup_mutex, then reads rdtgroup::flags.
> Between those two operations a concurrent caller of free_all_child_rdtgrp()
> or rmdir_all_sub() (which holds the mutex) can observe waitcount == 0 via
> atomic_read(), call rdtgroup_remove(), and kfree() the structure. The
> subsequent read of rdtgroup::flags in rdtgroup_kn_put() is then a
> use-after-free, and the structure may even be freed twice if the freed
> memory happens to satisfy the RDT_DELETED flag check.
> 
> Replace the bare atomic_dec_and_test() with atomic_dec_and_mutex_lock()
> so that the decrement-to-zero takes rdtgroup_mutex before the count
> becomes globally visible. The inspection of rdtgroup::flags then runs
> under the same mutex held by the bulk freers, making the two paths
> mutually exclusive. The common case where the count does not reach
> zero remains lock-free. Defer kernfs_unbreak_active_protection() until
> after the mutex is dropped since kernfs active protections functionally
> wrap rdtgroup_mutex. Remove resource group, which in turn drops its kernfs
> reference, after kernfs protection is restored.
> 
> Fixes: b8511ccc75c0 ("x86/resctrl: Fix use-after-free when deleting resource groups")
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://sashiko.dev/#/patchset/20260515193944.15114-1-tony.luck%40intel.com?part=1
> Assisted-by: GitHub_Copilot:gemini-3.1-pro
> Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>

Seems good to me.

Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>

> ---
> Changes since V2:
> - New patch
> ---
>  fs/resctrl/rdtgroup.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index d323b515060b..6418395877cf 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -2606,15 +2606,24 @@ static void rdtgroup_kn_get(struct rdtgroup *rdtgrp, struct kernfs_node *kn)
>  
>  static void rdtgroup_kn_put(struct rdtgroup *rdtgrp, struct kernfs_node *kn)
>  {
> -	if (atomic_dec_and_test(&rdtgrp->waitcount) &&
> -	    (rdtgrp->flags & RDT_DELETED)) {
> +	bool needs_free;
> +
> +	if (!atomic_dec_and_mutex_lock(&rdtgrp->waitcount, &rdtgroup_mutex)) {
> +		kernfs_unbreak_active_protection(kn);
> +		return;
> +	}
> +
> +	needs_free = rdtgrp->flags & RDT_DELETED;
> +
> +	mutex_unlock(&rdtgroup_mutex);
> +
> +	kernfs_unbreak_active_protection(kn);
> +
> +	if (needs_free) {
>  		if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP ||
>  		    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED)
>  			rdtgroup_pseudo_lock_remove(rdtgrp);
> -		kernfs_unbreak_active_protection(kn);
>  		rdtgroup_remove(rdtgrp);
> -	} else {
> -		kernfs_unbreak_active_protection(kn);
>  	}
>  }
>