From: Tony Luck <tony.luck@intel.com> To: Borislav Petkov <bp@alien8.de> Cc: "Tony Luck" <tony.luck@intel.com>, x86@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, "Andy Lutomirski" <luto@kernel.org>, "Aili Yao" <yaoaili@kingsoft.com>, "HORIGUCHI NAOYA( 堀口 直也)" <naoya.horiguchi@nec.com> Subject: [PATCH 4/4] x86/mce: Avoid infinite loop for copy from user recovery Date: Thu, 25 Mar 2021 17:02:35 -0700 [thread overview] Message-ID: <20210326000235.370514-5-tony.luck@intel.com> (raw) In-Reply-To: <20210326000235.370514-1-tony.luck@intel.com> Recovery action when get_user() triggers a machine check uses the fixup path to make get_user() return -EFAULT. Also queue_task_work() sets up so that kill_me_maybe() will be called on return to user mode to send a SIGBUS to the current process. But there are places in the kernel where the code assumes that this EFAULT return was simply because of a page fault. The code takes some action to fix that, and then retries the access. This results in a second machine check. While processing this second machine check queue_task_work() is called again. But since this uses the same callback_head structure that was used in the first call, the net result is an entry on the current->task_works list that points to itself. When task_work_run() is called it loops forever in this code: do { next = work->next; work->func(work); work = next; cond_resched(); } while (work); Add a counter (current->mce_count) to keep track of repeated machine checks before task_work() is called. First machine check saves the address information and calls task_work_add(). Subsequent machine checks before that task_work call back is executed check that the address is in the same page as the first machine check (since the callback will offline exactly one page). Expected worst case is two machine checks before moving on (e.g. one user access with page faults disabled, then a repeat to the same addrsss with page faults enabled). Just in case there is some code that loops forever enforce a limit of 10. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/mce/core.c | 40 ++++++++++++++++++++++++++-------- include/linux/sched.h | 1 + 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 1570310cadab..999fd7f0330b 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -1250,6 +1250,9 @@ static void __mc_scan_banks(struct mce *m, struct pt_regs *regs, struct mce *fin static void kill_me_now(struct callback_head *ch) { + struct task_struct *p = container_of(ch, struct task_struct, mce_kill_me); + + p->mce_count = 0; force_sig(SIGBUS); } @@ -1258,6 +1261,7 @@ static void kill_me_maybe(struct callback_head *cb) struct task_struct *p = container_of(cb, struct task_struct, mce_kill_me); int flags = MF_ACTION_REQUIRED; + p->mce_count = 0; pr_err("Uncorrected hardware memory error in user-access at %llx", p->mce_addr); if (!p->mce_ripv) @@ -1277,18 +1281,36 @@ static void kill_me_never(struct callback_head *cb) { struct task_struct *p = container_of(cb, struct task_struct, mce_kill_me); + p->mce_count = 0; pr_err("Kernel accessed poison in user space at %llx\n", p->mce_addr); if (!memory_failure(p->mce_addr >> PAGE_SHIFT, 0)) set_mce_nospec(p->mce_addr >> PAGE_SHIFT, p->mce_whole_page); } -static void queue_task_work(struct mce *m, void (*func)(struct callback_head *)) +static void queue_task_work(struct mce *m, char *msg, void (*func)(struct callback_head *)) { - current->mce_addr = m->addr; - current->mce_kflags = m->kflags; - current->mce_ripv = !!(m->mcgstatus & MCG_STATUS_RIPV); - current->mce_whole_page = whole_page(m); - current->mce_kill_me.func = func; + int count = ++current->mce_count; + + /* First call, save all the details */ + if (count == 1) { + current->mce_addr = m->addr; + current->mce_kflags = m->kflags; + current->mce_ripv = !!(m->mcgstatus & MCG_STATUS_RIPV); + current->mce_whole_page = whole_page(m); + current->mce_kill_me.func = func; + } + + /* Ten is likley overkill. Don't expect more than two faults before task_work() */ + if (count > 10) + mce_panic("Too many machine checks while accessing user data", m, msg); + + /* Second or later call, make sure page address matches the one from first call */ + if (count > 1 && (current->mce_addr >> PAGE_SHIFT) != (m->addr >> PAGE_SHIFT)) + mce_panic("Machine checks to different user pages", m, msg); + + /* Do not call task_work_add() more than once */ + if (count > 1) + return; task_work_add(current, ¤t->mce_kill_me, TWA_RESUME); } @@ -1427,9 +1449,9 @@ noinstr void do_machine_check(struct pt_regs *regs) BUG_ON(!on_thread_stack() || !user_mode(regs)); if (kill_current_task) - queue_task_work(&m, kill_me_now); + queue_task_work(&m, msg, kill_me_now); else - queue_task_work(&m, kill_me_maybe); + queue_task_work(&m, msg, kill_me_maybe); } else { /* @@ -1447,7 +1469,7 @@ noinstr void do_machine_check(struct pt_regs *regs) } if (m.kflags & MCE_IN_KERNEL_COPYIN) - queue_task_work(&m, kill_me_never); + queue_task_work(&m, msg, kill_me_never); } out: mce_wrmsrl(MSR_IA32_MCG_STATUS, 0); diff --git a/include/linux/sched.h b/include/linux/sched.h index 2d213b52730c..8f9dc91498cf 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1364,6 +1364,7 @@ struct task_struct { mce_whole_page : 1, __mce_reserved : 62; struct callback_head mce_kill_me; + int mce_count; #endif #ifdef CONFIG_KRETPROBES -- 2.29.2
next prev parent reply other threads:[~2021-03-26 0:02 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-26 0:02 [RFC 0/4] Fix machine check recovery for copy_from_user Tony Luck 2021-03-26 0:02 ` [PATCH 1/4] x86/mce: Fix copyin code to return -EFAULT on machine check Tony Luck 2021-04-06 19:24 ` Borislav Petkov 2021-03-26 0:02 ` [PATCH 2/4] mce/iter: Check for copyin failure & return error up stack Tony Luck 2021-03-26 0:02 ` [PATCH 3/4] mce/copyin: fix to not SIGBUS when copying from user hits poison Tony Luck 2021-04-07 21:18 ` Borislav Petkov 2021-04-07 21:43 ` Luck, Tony 2021-04-08 8:49 ` Borislav Petkov 2021-04-08 17:08 ` Luck, Tony 2021-04-13 10:07 ` Borislav Petkov 2021-04-13 16:13 ` Luck, Tony 2021-04-14 13:05 ` Borislav Petkov 2021-03-26 0:02 ` Tony Luck [this message] 2021-04-08 13:36 ` [PATCH 4/4] x86/mce: Avoid infinite loop for copy from user recovery Borislav Petkov 2021-04-08 16:06 ` Luck, Tony 2021-04-08 2:13 ` [RFC 0/4] Fix machine check recovery for copy_from_user Aili Yao 2021-04-08 14:39 ` Luck, Tony 2021-04-09 6:49 ` Aili Yao 2021-04-19 21:28 [PATCH 4/4] x86/mce: Avoid infinite loop for copy from user recovery Jue Wang 2021-04-19 21:41 ` 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=20210326000235.370514-5-tony.luck@intel.com \ --to=tony.luck@intel.com \ --cc=bp@alien8.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=luto@kernel.org \ --cc=naoya.horiguchi@nec.com \ --cc=x86@kernel.org \ --cc=yaoaili@kingsoft.com \ --subject='Re: [PATCH 4/4] x86/mce: Avoid infinite loop for copy from user recovery' \ /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
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).