From: Borislav Petkov <bp@alien8.de>
To: Tony Luck <tony.luck@intel.com>
Cc: Youquan Song <youquan.song@intel.com>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 7/7] x86/mce: Decode a kernel instruction to determine if it is copying from user
Date: Mon, 5 Oct 2020 18:31:30 +0200 [thread overview]
Message-ID: <20201005163130.GD21151@zn.tnic> (raw)
In-Reply-To: <20200930232611.15355-8-tony.luck@intel.com>
On Wed, Sep 30, 2020 at 04:26:11PM -0700, Tony Luck wrote:
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index 9713825e6745..60bacf6e0501 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1236,14 +1236,19 @@ static void kill_me_maybe(struct callback_head *cb)
> if (!p->mce_ripv)
> flags |= MF_MUST_KILL;
>
> - if (!memory_failure(p->mce_addr >> PAGE_SHIFT, flags)) {
> + if (!memory_failure(p->mce_addr >> PAGE_SHIFT, flags) &&
> + !(p->mce_kflags & MCE_IN_KERNEL_COPYIN)) {
> set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page);
> sync_core();
> return;
> }
>
> - pr_err("Memory error not recovered");
> - kill_me_now(cb);
> + if (p->mce_vaddr != (void __user *)~0ul) {
As previously pointed out, pls test against -1L even if it is the
same value so that it is obvious this is the error value coming from
insn_get_addr_ref().
> + force_sig_mceerr(BUS_MCEERR_AR, p->mce_vaddr, PAGE_SHIFT);
> + } else {
> + pr_err("Memory error not recovered");
> + kill_me_now(cb);
> + }
> }
>
> /*
> diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
> index 8517cbf7b184..6e8b38cf52d9 100644
> --- a/arch/x86/kernel/cpu/mce/severity.c
> +++ b/arch/x86/kernel/cpu/mce/severity.c
> @@ -10,6 +10,9 @@
> #include <linux/init.h>
> #include <linux/debugfs.h>
> #include <asm/mce.h>
> +#include <asm/traps.h>
> +#include <asm/insn.h>
> +#include <asm/insn-eval.h>
> #include <linux/uaccess.h>
>
> #include "internal.h"
> @@ -198,6 +201,45 @@ static struct severity {
> #define mc_recoverable(mcg) (((mcg) & (MCG_STATUS_RIPV|MCG_STATUS_EIPV)) == \
> (MCG_STATUS_RIPV|MCG_STATUS_EIPV))
>
> +static bool is_copy_from_user(struct pt_regs *regs)
> +{
> + u8 insn_buf[MAX_INSN_SIZE];
> + struct insn insn;
> + unsigned long addr;
> +
> + if (copy_from_kernel_nofault(insn_buf, (void *)regs->ip, MAX_INSN_SIZE))
> + return false;
> +
> + kernel_insn_init(&insn, insn_buf, MAX_INSN_SIZE);
> + insn_get_opcode(&insn);
> + if (!insn.opcode.got)
> + return false;
> +
> + switch (insn.opcode.value) {
> + /* MOV mem,reg */
> + case 0x8A: case 0x8B:
> + /* MOVZ mem,reg */
> + case 0xB60F: case 0xB70F:
> + insn_get_modrm(&insn);
> + insn_get_sib(&insn);
You need to test here:
insn->modrm.got = 1;
and
insn->sib.got = 1;
I know, this is weird - those functions should return an error value
instead of being void and I've asked Masami in the past but no reply.
Who knows, one fine day I might convert the crap to do that instead.
> + addr = (unsigned long)insn_get_addr_ref(&insn, regs);
> + break;
> + /* REP MOVS */
> + case 0xA4: case 0xA5:
> + addr = regs->si;
> + break;
> + default:
> + return false;
> + }
> +
> + if (fault_in_kernel_space(addr))
> + return false;
> +
> + current->mce_vaddr = (void __user *)addr;
> +
> + return true;
> +}
> +
> /*
> * If mcgstatus indicated that ip/cs on the stack were
> * no good, then "m->cs" will be zero and we will have
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2020-10-05 16:31 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200908175519.14223-1-tony.luck@intel.com>
2020-09-08 17:55 ` [PATCH 1/8] x86/mce: Stop mce_reign() from re-computing severity for every CPU Tony Luck
2020-09-14 17:21 ` Borislav Petkov
2020-09-14 17:32 ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-09-08 17:55 ` [PATCH 4/8] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-09-16 9:59 ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 5/8] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-09-16 10:53 ` Borislav Petkov
2020-09-16 19:26 ` Luck, Tony
2020-09-17 17:04 ` Borislav Petkov
2020-09-17 21:57 ` Luck, Tony
2020-09-18 7:51 ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 6/8] x86/mce: Change fault_in_kernel_space() from static to global Tony Luck
2020-09-08 17:55 ` [PATCH 7/8] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-09-18 16:13 ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 8/8] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-09-21 11:31 ` Borislav Petkov
2020-09-30 23:26 ` [PATCH v2 0/7] Add machine check recovery when copying from user space Tony Luck
2020-09-30 23:26 ` [PATCH v2 1/7] x86/mce: Pass pointer to saved pt_regs to severity calculation routines Tony Luck
2020-09-30 23:26 ` [PATCH v2 2/7] x86/mce: Provide method to find out the type of exception handle Tony Luck
2020-10-05 16:35 ` Borislav Petkov
2020-09-30 23:26 ` [PATCH v2 3/7] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-10-05 16:34 ` Borislav Petkov
2020-09-30 23:26 ` [PATCH v2 4/7] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-09-30 23:26 ` [PATCH v2 5/7] x86/mce: Change fault_in_kernel_space() from static to global Tony Luck
2020-10-05 16:33 ` Borislav Petkov
2020-09-30 23:26 ` [PATCH v2 6/7] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-10-05 16:32 ` Borislav Petkov
2020-10-05 17:47 ` Luck, Tony
2020-09-30 23:26 ` [PATCH v2 7/7] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-10-05 16:31 ` Borislav Petkov [this message]
2020-10-06 21:09 ` [PATCH v3 0/6] Add machine check recovery when copying from user space Tony Luck
2020-10-06 21:09 ` [PATCH v3 1/6] x86/mce: Pass pointer to saved pt_regs to severity calculation routines Tony Luck
2020-10-07 10:02 ` [tip: ras/core] " tip-bot2 for Youquan Song
2020-10-06 21:09 ` [PATCH v3 2/6] x86/mce: Provide method to find out the type of exception handle Tony Luck
2020-10-07 10:02 ` [tip: ras/core] x86/mce: Provide method to find out the type of an exception handler tip-bot2 for Tony Luck
2020-10-06 21:09 ` [PATCH v3 3/6] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-10-07 10:02 ` [tip: ras/core] " tip-bot2 for Youquan Song
2020-10-06 21:09 ` [PATCH v3 4/6] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-10-07 8:23 ` David Laight
2020-10-07 18:49 ` Luck, Tony
2020-10-07 21:11 ` David Laight
2020-10-07 10:02 ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-10-06 21:09 ` [PATCH v3 5/6] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-10-07 10:02 ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-10-06 21:09 ` [PATCH v3 6/6] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-10-07 10:02 ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-09-09 15:05 ` [RESEND PATCH 0/8] Add machine check recovery when copying from user space Tony Luck
[not found] ` <20200908175519.14223-4-tony.luck@intel.com>
2020-09-15 9:11 ` [PATCH 3/8] x86/mce: Provide method to find out the type of exception handle Borislav Petkov
2020-09-15 16:24 ` Luck, Tony
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201005163130.GD21151@zn.tnic \
--to=bp@alien8.de \
--cc=linux-kernel@vger.kernel.org \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
--cc=youquan.song@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).