linux-kernel - Re: [PATCH] x86/resctrl: Don't stop walking closids when a locksetup group is found

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] [day] [month] [year] [list]

Date:   Thu, 30 May 2019 11:16:00 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     James Morse <james.morse@....com>, linux-kernel@...r.kernel.org,
        x86@...nel.org
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Avin <hpa@...or.com>
Subject: Re: [PATCH] x86/resctrl: Don't stop walking closids when a locksetup
 group is found

Hi James,

On 5/30/2019 8:57 AM, James Morse wrote:
> When a new control group is created __init_one_rdt_domain() walks all
> the other closids to calculate the sets of used and unused bits.
> 
> If it discovers a pseudo_locksetup group, it breaks out of the loop.
> This means any later closid doesn't get its used bits added to used_b.
> These bits will then get set in unused_b, and added to the new
> control group's configuration, even if they were marked as exclusive
> for a later closid.
> 
> When encountering a pseudo_locksetup group, we should continue. This
> is because "a resource group enters 'pseudo-locked' mode after the
> schemata is written while the resource group is in 'pseudo-locksetup'
> mode." When we find a pseudo_locksetup group, its configuration is
> expected to be overwritten, we can skip it.
> 
> Fixes: dfe9674b04ff6 ("x86/intel_rdt: Enable entering of pseudo-locksetup mode")
> Signed-off-by: James Morse <james.morse@....com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 333c177a2471..049ccb709957 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2541,8 +2541,15 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
>  	for (i = 0; i < closids_supported(); i++, ctrl++) {
>  		if (closid_allocated(i) && i != closid) {
>  			mode = rdtgroup_mode_by_closid(i);
> -			if (mode == RDT_MODE_PSEUDO_LOCKSETUP)
> -				break;
> +			if (mode == RDT_MODE_PSEUDO_LOCKSETUP) {
> +				/*
> +				 * ctrl values for locksetup aren't relevant
> +				 * until the schemata is written, and the mode
> +				 * becomes RDT_MODE_PSEUDO_LOCKED.
> +				 */
> +				continue;
> +			}
> +
>  			/*
>  			 * If CDP is active include peer domain's
>  			 * usage to ensure there is no overlap
> 

Thank you very much for catching this. The only possible quibbles that
will be guided by the maintainer's style preference are the unnecessary
braces and extra empty line, but this fix is needed. Also, could "Cc:
stable@...r.kernel.org" please be added to the sign-off area?

Acked-by: Reinette Chatre <reinette.chatre@...el.com>

Reinette