linux-kernel - Re: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits

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]

Date:   Wed, 22 Apr 2020 16:21:51 +0100
From:   John Haxby <john.haxby@...cle.com>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, Jonathan Corbet <corbet@....net>,
        x86@...nel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/1] x86/fpu: Allow clearcpuid= to clear several bits

> On 22 Apr 2020, at 15:35, Andi Kleen <ak@...ux.intel.com> wrote:
> 
> 
> Thanks good catch.
> 
>> 	if (cmdline_find_option(boot_command_line, "clearcpuid", arg,
>> -				sizeof(arg)) &&
>> -	    get_option(&argptr, &bit) &&
>> -	    bit >= 0 &&
>> -	    bit < NCAPINTS * 32)
>> -		setup_clear_cpu_cap(bit);
>> +				sizeof(arg))) {
>> +		/* cpuid bit numbers are mostly three digits */
>> +		enum  { nints = sizeof(arg)/(3+1) + 1 };
> 
> Not sure what the digits have to do with the stack space of an int array.
> 
> We should have enough stack to afford some more than 8.

sizeof(arg) == 32; room enough for eight three-digit with their trailing commas.   If sizeof(arg) == 1024 instead then there'd be more than enough room to remove every single feature.   TBH, 512 is more than enough for the 89 flags I have listed on this machine I'm looking at here.   I'll grow sizeof(arg) and nints accordingly.

> 
> Would be good to have a warning if the arguments are longer.
> 

Yes, I should definitely do that -- coming to a V2 soon.

> Maybe it would be simpler to fix the early arg parser
> to allow multiple instances again? That would also avoid the limit,
> and keep everything compatible.
> 

I did wonder about that.   However, cmdline_find_option() is specifically documented as 

 * Find a non-boolean option (i.e. option=argument). In accordance with
 * standard Linux practice, if this option is repeated, this returns the
 * last instance on the command line.

And since that appeared in 2017 I decided to stick with the new-fangled interface :)   This is a little-used feature; I'm not sure it's worth the effort of parsing the command line for the old style.  What do you think?

jch

> -Andi
> 
> 
>> +		int i, bits[nints];
>> +
>> +		get_options(arg, nints, bits);
>> +		for (i = 1; i <= bits[0]; i++) {
>> +			if (bits[i] >= 0 && bits[i] < NCAPINTS * 32)
>> +				setup_clear_cpu_cap(bits[i]);
>> +		}
>> +	}
>> }
>> 
>> /*
>> -- 
>> 2.25.3
>>