linux-kernel - Re: [PATCH] fs/resctrl: Restore the missing rdt_last_cmd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c94b97ea-4dd4-7575-2144-81e4272c8fee@huawei.com>
Date: Fri, 30 May 2025 17:34:28 +0800
From: Zeng Heng <zengheng4@...wei.com>
To: Reinette Chatre <reinette.chatre@...el.com>
CC: <bobo.shaobowang@...wei.com>, <linux-kernel@...r.kernel.org>,
	<Dave.Martin@....com>, <tony.luck@...el.com>, <xiaochen.shen@...el.com>,
	<bp@...e.de>, <fenghua.yu@...el.com>, <james.morse@....com>
Subject: Re: [PATCH] fs/resctrl: Restore the missing rdt_last_cmd_clear()



On 2025/5/30 6:01, Reinette Chatre wrote:
> Hi Zeng Heng,
> 
> Thank you very much for catching this and providing a fix.
> 
> On 5/29/25 4:33 AM, Zeng Heng wrote:
>> The fixes tag patch resolves the lockdep warning. However, directly
>> removing rdt_last_cmd_clear() would leave the last_cmd_status interface
>> with stale logs, which does not conform to the functional definition before
>> the fix. Therefore, the rdt_last_cmd_clear() operation is performed after
>> successfully acquiring the rdtgroup_mutex.
> 
> I would like to suggest some rework to changelog to meet requirements from
> Documentation/process/maintainer-tip.rst. Specifically the rules about
> imperative tone and structure of the changelog. Below attempts to address
> those requirements but please feel free to rework after you considered the
> rules yourself:
> 
> 	A lockdep fix removed two rdt_last_cmd_clear() calls that were used
> 	to clear the last_cmd_status buffer but called without holding the
> 	required rdtgroup_mutex. The impacted resctrl commands are:
> 	writing to the cpus or cpus_list files and creating a new monitor
> 	or control group. With stale data in the last_cmd_status buffer the
> 	impacted resctrl commands report the stale error on success, or append
> 	its own failure message to the stale error on failure.
> 
> 	Restore the rdt_last_cmd_clear() calls after acquiring rdtgroup_mutex.

Thank you for the correction, I will review the requirements mentioned
in the documents above.

> 
>>
>> Fixes: c8eafe149530 ("x86/resctrl: Fix potential lockdep warning")
>> Signed-off-by: Zeng Heng <zengheng4@...wei.com>
>> ---
>>   fs/resctrl/rdtgroup.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index cc37f58b47dd..4aae9eb74215 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -536,6 +536,8 @@ static ssize_t rdtgroup_cpus_write(struct kernfs_open_file *of,
>>   		goto unlock;
>>   	}
>>   
>> +	rdt_last_cmd_clear();
>> +
>>   	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
>>   	    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
>>   		ret = -EINVAL;
>> @@ -3481,6 +3483,8 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
>>   		goto out_unlock;
>>   	}
>>   
>> +	rdt_last_cmd_clear();
>> +
> 
> Could you please move this to be right after acquiring the mutex? I think clearing
> last_cmd_status at beginning of a resctrl command's work is a good pattern to follow.
> Thus a change like:

The patch will be corrected in version v2. Thank you again.

Best regards，
Zeng Heng