Re: [PATCH 2/2] x86/bugs: spectre user default must depend on MITIGATION_SPECTRE_V2

[Breno Leitao]

Hello Pawan,

On Mon, Oct 28, 2024 at 07:34:53AM -0700, Pawan Gupta wrote:
> On Tue, Oct 15, 2024 at 03:51:06AM -0700, Breno Leitao wrote:
> > @@ -1277,9 +1277,13 @@ static __ro_after_init enum spectre_v2_mitigation_cmd spectre_v2_cmd;
> >  static enum spectre_v2_user_cmd __init
> >  spectre_v2_parse_user_cmdline(void)
> >  {
> > +	enum spectre_v2_user_cmd mode;
> >  	char arg[20];
> >  	int ret, i;
> >  
> > +	mode = IS_ENABLED(CONFIG_MITIGATION_SPECTRE_V2) ?
> > +		SPECTRE_V2_USER_CMD_AUTO : SPECTRE_V2_USER_CMD_NONE;
> > +
> >  	switch (spectre_v2_cmd) {
> >  	case SPECTRE_V2_CMD_NONE:
> >  		return SPECTRE_V2_USER_CMD_NONE;
> > @@ -1292,7 +1296,7 @@ spectre_v2_parse_user_cmdline(void)
> >  	ret = cmdline_find_option(boot_command_line, "spectre_v2_user",
> >  				  arg, sizeof(arg));
> >  	if (ret < 0)
> > -		return SPECTRE_V2_USER_CMD_AUTO;
> > +		return mode;
> 
> This doesn't look right to me, spectre_v2=eibrs|retpoline... will override
> CONFIG_MITIGATION_SPECTRE_V2=n and enable the kernel mitigation, but the
> user mitigation will stay disabled.

Correct. In the current configuration, if the user compiles the kernel
with CONFIG_MITIGATION_SPECTRE_V2=n explict, then the kernel will have
all spectre v2 mitigations disable by default, and can opt-in using
command line arguments.

In other words, if the user compiles the kernel with
CONFIG_MITIGATION_SPECTRE_V2=n, then it means all spectre v2 mitigations
will be disable. the user wants to opt-in mitigation by mitigation, tho.

For instance, if user sets `spectre_v2=eibrs|retpoline` in cmdline, then
it must not set `spectre_v2_user` automatically. User can decide to do
enable userspace mitigation so as well by passing userspace another
command line `spectre_v2_user=on|prctl|..`. I.e, they are independent
selectable.

> If this is the intention it should be
> clearly documented that enabling kernel mitigation does not enable user
> mitigation. And an explicit spectre_v2_user= is required to enable user
> mitigation.

That is fair. I didn't find a place where to document about diferent
behavior when CONFIG_MITIGATION_X is disabled. What would you suggest?

Thanks for review and good points.
--breno