Messages in this thread Patch in this message | | | Date | Tue, 26 Mar 2019 00:40:34 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH 12/27] x86/msr: Restrict MSR access when the kernel is locked down |
| |
Matthew,
On Mon, 25 Mar 2019, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org> > > Writing to MSRs should not be allowed if the kernel is locked down, since > it could lead to execution of arbitrary code in kernel mode. Based on a > patch by Kees Cook. > > MSR accesses are logged for the purposes of building up a whitelist as per > Alan Cox's suggestion. > > Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org> > Signed-off-by: David Howells <dhowells@redhat.com> > Acked-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
I'm pretty sure, that I reviewed a different version of this, but due to the lack of:
1) A version indicator in the subject line, i.e. [PATCH v7 12/27]
2) A simple change indicator after the --- separator, e.g.
v6 -> v7: Add MRS logging to dmesg .....
It's tedious to figure out what actually changed here. I just know for sure that the printk wasn't there before.
It's not a huge effort adding such information, but it's very helpful for those who are supposed to look at your patches. Those people are drowned in patches so making it as easy as it goes would be very appreciated.
> +++ b/arch/x86/kernel/msr.c > @@ -84,6 +84,11 @@ static ssize_t msr_write(struct file *file, const char __user *buf, > int err = 0; > ssize_t bytes = 0; > > + if (kernel_is_locked_down("Direct MSR access")) { > + pr_info("Direct access to MSR %x\n", reg);
I'm really not fond of this at all. /dev/msr should simply die.
Maintaining a whitelist for this is a horrible idea as you will get a gazillion of excuses why access to a particular MSR is sane. And I'm neither interested in these discussions nor interested in adding the whitelist to this trainwreck,
The right thing to do is to provide sane interfaces and that's where we are moving to. So simply blocking the access with locked down mode might be very helpful to accelerate that.
Thanks,
tglx
| |