linux-kernel - Re: [PATCH v6 31/42] x86/resctrl: Remove the limit on the number of CLOSID

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: <3e21aab4-d813-4f5b-9f60-74887270f12f@arm.com>
Date: Fri, 28 Feb 2025 19:53:54 +0000
From: James Morse <james.morse@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, x86@...nel.org,
 linux-kernel@...r.kernel.org
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, H Peter Anvin <hpa@...or.com>,
 Babu Moger <Babu.Moger@....com>, shameerali.kolothum.thodi@...wei.com,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
 Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>,
 Shanker Donthineni <sdonthineni@...dia.com>
Subject: Re: [PATCH v6 31/42] x86/resctrl: Remove the limit on the number of
 CLOSID

Hi Reinette,

On 20/02/2025 04:21, Reinette Chatre wrote:
> On 2/7/25 10:18 AM, James Morse wrote:
>> From: Amit Singh Tomar <amitsinght@...vell.com>
>>
>> Resctrl allocates and finds free CLOSID values using the bits of a u32.
>> This restricts the number of control groups that can be created by
>> user-space.
>>
>> MPAM has an architectural limit of 2^16 CLOSID values, Intel x86 could
>> be extended beyond 32 values. There is at least one MPAM platform which
>> supports more than 32 CLOSID values.
>>
>> Replace the fixed size bitmap with calls to the bitmap API to allocate
>> an array of a sufficient size.
>>
>> ffs() returns '1' for bit 0, hence the existing code subtracts 1 from
>> the index to get the CLOSID value. find_first_bit() returns the bit
>> number which does not need adjusting.
>>
>> Signed-off-by: Amit Singh Tomar <amitsinght@...vell.com>
>> [ morse: fixed the off-by-one in the allocator and the wrong
>>  not-found value. Removed the limit. Rephrase the commit message. ]
>> Signed-off-by: James Morse <james.morse@....com>

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 08fec23a38bf..de79da30d500 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c

>> @@ -152,20 +152,30 @@ int closids_supported(void)

>> +static void closid_exit(void)
>> +{
>> +	bitmap_free(closid_free_map);

> With closid_free_map being a global, could this also set
> closid_free_map to NULL?

Makes sense,

> 
>>  }
>>  
>>  static int closid_alloc(void)

>> @@ -2754,20 +2763,22 @@ static int rdt_get_tree(struct fs_context *fc)
>>  		goto out_ctx;
>>  	}
>>  
>> -	closid_init();
>> +	ret = closid_init();
>> +	if (ret)
>> +		goto out_schemata_free;
>>  
>>  	if (resctrl_arch_mon_capable())
>>  		flags |= RFTYPE_MON;
>>  
>>  	ret = rdtgroup_add_files(rdtgroup_default.kn, flags);
>>  	if (ret)
>> -		goto out_schemata_free;
>> +		goto out_closid_exit;
>>  
>>  	kernfs_activate(rdtgroup_default.kn);
>>  
>>  	ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
>>  	if (ret < 0)
>> -		goto out_schemata_free;
>> +		goto out_closid_exit;
>>  
>>  	if (resctrl_arch_mon_capable()) {
>>  		ret = mongroup_create_dir(rdtgroup_default.kn,

> With closid_init() called from rdt_get_tree() during mount I expected
> closid_exit() to be called from rdt_kill_sb() during unmount ?

Ah, I'd missed that - the old version got called multiple times but it was harmless. Now
it potentially leaks the bitmap. Fixed.


Thanks!

James