Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
On Sun, Apr 11, 2021 at 04:21:21PM -0700, Andy Lutomirski wrote: > https://bugs.winehq.org/show_bug.cgi?id=33275#c19 > > I sure hope no one is still doing this. Aha, IRET with rFLAGS.NT set. At least it is only an ad-hoc program to fix this particular issue and I hope too it hasn't propagated somewhere else. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
On Sun, Apr 11, 2021 at 10:04 AM Borislav Petkov wrote: > > On Sun, Apr 11, 2021 at 09:57:20AM -0700, Andy Lutomirski wrote: > > Working around a kernel bug. The workaround only worked on AMD > > systems. The correct solution was to fix the kernel bug, not poke > > MSRs. > > Do you remember which program(s) and where I can get them to have a > look? > https://bugs.winehq.org/show_bug.cgi?id=33275#c19 I sure hope no one is still doing this. --Andy
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
On Sun, Apr 11, 2021 at 09:57:20AM -0700, Andy Lutomirski wrote: > Working around a kernel bug. The workaround only worked on AMD > systems. The correct solution was to fix the kernel bug, not poke > MSRs. Do you remember which program(s) and where I can get them to have a look? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
> On Apr 11, 2021, at 9:43 AM, Andi Kleen wrote: > > >> >> I have actually seen real user programs poke MSR_SYSCALL_MASK. > > Hmm, what was the use case? > > Working around a kernel bug. The workaround only worked on AMD systems. The correct solution was to fix the kernel bug, not poke MSRs.
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
> I have actually seen real user programs poke MSR_SYSCALL_MASK. Hmm, what was the use case? -Andi
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
On Sat, Apr 10, 2021 at 11:52 AM Andi Kleen wrote: > > Borislav Petkov writes: > > > From: Borislav Petkov > > Date: Sat, 10 Apr 2021 14:08:13 +0200 > > > > There are a bunch of MSRs which luserspace has no business poking at, > > whatsoever. Add a ban list and put the TSC-related MSRs in there. Issue > > a big juicy splat to catch offenders. > > Have you ever seen any user programs actually write those MSRs? > I don't see why they ever would, it's not that they have any motivation > to do it (unlike SMM), and I don't know of any examples. I have actually seen real user programs poke MSR_SYSCALL_MASK.
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
On Sat, Apr 10, 2021 at 11:52:17AM -0700, Andi Kleen wrote: > Have you ever seen any user programs actually write those MSRs? > I don't see why they ever would, it's not that they have any motivation > to do it (unlike SMM), and I don't know of any examples. You'd be surprised how much motivation people have to poke at random MSRs. Just browse some of those tools on github which think poking at MSRs is ok. > The whole MSR blocking seems more like a tilting at windmills > type effort. Nope, this is trying to salvage the current situation of people thinking it is a good idea to poke at random MSRs and develop all kinds of tools around it and then run those tools ontop of a kernel which pokes at those same MSRs. It is not really hard to realize that touching resources in an unsynchronized way is a disaster waiting to happen. No matter how useful and how wonderful those tools are. > But on a non locked down system fully accessible MSRs are really > useful for all kind of debugging and tuning, and anything that > prevents that is bad. If you wanna do that, you can just as well patch your kernel. We're currently tainting the kernel on MSR writes and perhaps that's good enough for now but if some tool starts doing dumb crap and pokes at MSRs it should not be poking at and users start complaining because of it, I'm committing that. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
Borislav Petkov writes: > From: Borislav Petkov > Date: Sat, 10 Apr 2021 14:08:13 +0200 > > There are a bunch of MSRs which luserspace has no business poking at, > whatsoever. Add a ban list and put the TSC-related MSRs in there. Issue > a big juicy splat to catch offenders. Have you ever seen any user programs actually write those MSRs? I don't see why they ever would, it's not that they have any motivation to do it (unlike SMM), and I don't know of any examples. The whole MSR blocking seems more like a tilting at windmills type effort. Root kits typically write from the kernel anyways. And the only results we have so far is various legitimate debug and benchmark utilities running much slower due to them flooding the kernel log with warnings. I can see that there are security reasons to lock down MSRs, but that is already handled fine with existing sandbox and lockdown mechanisms. But on a non locked down system fully accessible MSRs are really useful for all kind of debugging and tuning, and anything that prevents that is bad. -Andi
Re: [PATCH] x86/msr: Block writes to certain MSRs unconditionally
> On Apr 10, 2021, at 5:11 AM, Borislav Petkov wrote:
>
> From: Borislav Petkov
> Date: Sat, 10 Apr 2021 14:08:13 +0200
>
> There are a bunch of MSRs which luserspace has no business poking at,
> whatsoever. Add a ban list and put the TSC-related MSRs in there. Issue
> a big juicy splat to catch offenders.
Ack!
Can you add STAR, CSTAR, LSTAR, SYSENTER*, SYSCALL*, and EFER please?
>
> Signed-off-by: Borislav Petkov
> ---
> arch/x86/kernel/msr.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
> index ed8ac6bcbafb..574bd2c6d4f8 100644
> --- a/arch/x86/kernel/msr.c
> +++ b/arch/x86/kernel/msr.c
> @@ -78,6 +78,13 @@ static ssize_t msr_read(struct file *file, char __user
> *buf,
>return bytes ? bytes : err;
> }
>
> +static const u32 msr_ban_list[] = {
> +MSR_IA32_TSC,
> +MSR_TSC_AUX,
> +MSR_IA32_TSC_ADJUST,
> +MSR_IA32_TSC_DEADLINE,
> +};
> +
> static int filter_write(u32 reg)
> {
>/*
> @@ -89,6 +96,16 @@ static int filter_write(u32 reg)
> * avoid saturating the ring buffer.
> */
>static DEFINE_RATELIMIT_STATE(fw_rs, 30 * HZ, 1);
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(msr_ban_list); i++) {
> +if (msr_ban_list[i] != reg)
> +continue;
> +
> +WARN_ONCE(1, "Blocked write to MSR 0x%x\n", reg);
> +
> +return -EINVAL;
> +}
>
>switch (allow_writes) {
>case MSR_WRITES_ON: return 0;
> --
> 2.29.2
>
>
> --
> Regards/Gruss,
>Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
