Re: [PATCH v3 3/9] fs/resctrl: Fix use-after-free during unmount

[Reinette Chatre]

Hi Ben,

On 5/28/26 2:45 AM, Ben Horgan wrote:
> On 5/22/26 20:15, Reinette Chatre wrote:
..

>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index f573db9e6e84..8a1457825919 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -585,14 +585,20 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>>   *
>>   * On resource group creation via a mkdir, an extra kernfs_node reference is
>>   * taken to ensure that the rdtgroup structure remains accessible for the
>> - * rdtgroup_kn_unlock() calls where it is removed.
>> + * rdtgroup_kn_unlock() calls where it is removed. The default group is
>> + * statically allocated: it does not have an extra reference but will have
>> + * RDT_DELETED set on unmount to support safe access to its associated files
>> + * via rdtgroup_kn_lock_live/rdtgroup_kn_unlock().
>>   *
>> - * Drop the extra reference here, then free the rdtgroup structure.
>> + * For all but the default group: drop the extra reference, then free the
>> + * rdtgroup structure.
>>   *
>>   * Return: void
>>   */
>>  static void rdtgroup_remove(struct rdtgroup *rdtgrp)
>>  {
>> +	if (rdtgrp == &rdtgroup_default)
>> +		return;
>>  	kernfs_put(rdtgrp->kn);
>>  	kfree(rdtgrp);
>>  }
>> @@ -2975,6 +2981,7 @@ static void resctrl_fs_teardown(void)
>>  	mon_put_kn_priv();
>>  	rdt_pseudo_lock_release();
>>  	rdtgroup_default.mode = RDT_MODE_SHAREABLE;
>> +	rdtgroup_default.flags = RDT_DELETED;
> 
> Would it be better to use |= here?
> I expect it's not functionally different but I'm unsure of the interaction with
> RDT_DELETED_PLR and it seems best in general unless clearing other possible
> flags is intended.

Thank you very much for taking a look and for the other tags.

I see this initialization as assigning a "reset" value that reflects the
cleanup done by resctrl_fs_teardown() as opposed to recording/adding another state
during runtime.
If resctrl does add RDT_DELETED_PLR then there is no interaction to be
concerned about here since pseudo-locking is not supported on the default
group. You can confirm this by considering the first "if (rdtgrp == &rdtgroup_default)"
check within fs/resctrl/pseudo_lock.c:rdtgroup_locksetup_enter().

Since there is only one flag, RDT_DELETED, possible for the default group
you are correct that using |= is not functionally different but my preference
would be to use assignment in this spot just to make it clear that this is
a reset value.

Reinette