On Fri, Jun 12, 2020 at 12:50:26PM +0200, Borislav Petkov wrote:
> @@ -95,11 +114,18 @@ static ssize_t msr_write(struct file *file, const char
> __user *buf,
> err = wrmsr_safe_on_cpu(cpu, reg, data[0], data[1]);
> if (err)
> break;
> +
> tmp += 2;
> bytes += 8;
> }
>
> - return bytes ? bytes : err;
> + if (bytes) {
> + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
The kernel should be tainted if the WRMSR is attempted, regardless of
whether it succeeds, and it should happen before the WRMSR. E.g. pointing
MSR_IA32_DS_AREA at a bad address will likely cause an OOPS on the #PF
that would occur the next time the CPU attempts to access the area, which
could happen before wrmsr_safe_on_cpu() even returns. In general, there's
no telling what microcode magic is buried behind WRMSR, i.e. a fault on
WRMSR is not a good indicator that the CPU is still in a sane state.
It might also make sense to do pr_err/warn_ratelimited() before the WRMSR,
e.g. to help triage MSR writes that cause insta-death or lead to known bad
behavior down the line.
> +
> + return bytes;
> + }
> +
> + return err;
> }
>
> static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
> @@ -242,6 +268,8 @@ static void __exit msr_exit(void)
> }
> module_exit(msr_exit)
>
> +module_param(allow_writes, bool, 0400);
This can be 0600, or maybe 0644, i.e. allow the user to enable/disable
writes after the module has been loaded.
> +
> MODULE_AUTHOR("H. Peter Anvin <h...@zytor.com>");
> MODULE_DESCRIPTION("x86 generic MSR driver");
> MODULE_LICENSE("GPL");
