Skip to content

Commit 81065b3

Browse files
aeglsuryasaimadhu
authored andcommitted
x86/mce: Avoid infinite loop for copy from user recovery
There are two cases for machine check recovery: 1) The machine check was triggered by ring3 (application) code. This is the simpler case. The machine check handler simply queues work to be executed on return to user. That code unmaps the page from all users and arranges to send a SIGBUS to the task that triggered the poison. 2) The machine check was triggered in kernel code that is covered by an exception table entry. In this case the machine check handler still queues a work entry to unmap the page, etc. but this will not be called right away because the #MC handler returns to the fix up code address in the exception table entry. Problems occur if the kernel triggers another machine check before the return to user processes the first queued work item. Specifically, the work is queued using the ->mce_kill_me callback structure in the task struct for the current thread. Attempting to queue a second work item using this same callback results in a loop in the linked list of work functions to call. So when the kernel does return to user, it enters an infinite loop processing the same entry for ever. There are some legitimate scenarios where the kernel may take a second machine check before returning to the user. 1) Some code (e.g. futex) first tries a get_user() with page faults disabled. If this fails, the code retries with page faults enabled expecting that this will resolve the page fault. 2) Copy from user code retries a copy in byte-at-time mode to check whether any additional bytes can be copied. On the other side of the fence are some bad drivers that do not check the return value from individual get_user() calls and may access multiple user addresses without noticing that some/all calls have failed. Fix by adding 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 four machine checks before moving on (e.g. one user access with page faults disabled, then a repeat to the same address with page faults enabled ... repeat in copy tail bytes). Just in case there is some code that loops forever enforce a limit of 10. [ bp: Massage commit message, drop noinstr, fix typo, extend panic messages. ] Fixes: 5567d11 ("x86/mce: Send #MC singal from task work") Signed-off-by: Tony Luck <[email protected]> Signed-off-by: Borislav Petkov <[email protected]> Cc: <[email protected]> Link: https://lkml.kernel.org/r/YT/[email protected]
1 parent 34b1999 commit 81065b3

File tree

2 files changed

+33
-11
lines changed

2 files changed

+33
-11
lines changed

arch/x86/kernel/cpu/mce/core.c

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,6 +1253,9 @@ static void __mc_scan_banks(struct mce *m, struct pt_regs *regs, struct mce *fin
12531253

12541254
static void kill_me_now(struct callback_head *ch)
12551255
{
1256+
struct task_struct *p = container_of(ch, struct task_struct, mce_kill_me);
1257+
1258+
p->mce_count = 0;
12561259
force_sig(SIGBUS);
12571260
}
12581261

@@ -1262,6 +1265,7 @@ static void kill_me_maybe(struct callback_head *cb)
12621265
int flags = MF_ACTION_REQUIRED;
12631266
int ret;
12641267

1268+
p->mce_count = 0;
12651269
pr_err("Uncorrected hardware memory error in user-access at %llx", p->mce_addr);
12661270

12671271
if (!p->mce_ripv)
@@ -1290,17 +1294,34 @@ static void kill_me_maybe(struct callback_head *cb)
12901294
}
12911295
}
12921296

1293-
static void queue_task_work(struct mce *m, int kill_current_task)
1297+
static void queue_task_work(struct mce *m, char *msg, int kill_current_task)
12941298
{
1295-
current->mce_addr = m->addr;
1296-
current->mce_kflags = m->kflags;
1297-
current->mce_ripv = !!(m->mcgstatus & MCG_STATUS_RIPV);
1298-
current->mce_whole_page = whole_page(m);
1299+
int count = ++current->mce_count;
12991300

1300-
if (kill_current_task)
1301-
current->mce_kill_me.func = kill_me_now;
1302-
else
1303-
current->mce_kill_me.func = kill_me_maybe;
1301+
/* First call, save all the details */
1302+
if (count == 1) {
1303+
current->mce_addr = m->addr;
1304+
current->mce_kflags = m->kflags;
1305+
current->mce_ripv = !!(m->mcgstatus & MCG_STATUS_RIPV);
1306+
current->mce_whole_page = whole_page(m);
1307+
1308+
if (kill_current_task)
1309+
current->mce_kill_me.func = kill_me_now;
1310+
else
1311+
current->mce_kill_me.func = kill_me_maybe;
1312+
}
1313+
1314+
/* Ten is likely overkill. Don't expect more than two faults before task_work() */
1315+
if (count > 10)
1316+
mce_panic("Too many consecutive machine checks while accessing user data", m, msg);
1317+
1318+
/* Second or later call, make sure page address matches the one from first call */
1319+
if (count > 1 && (current->mce_addr >> PAGE_SHIFT) != (m->addr >> PAGE_SHIFT))
1320+
mce_panic("Consecutive machine checks to different user pages", m, msg);
1321+
1322+
/* Do not call task_work_add() more than once */
1323+
if (count > 1)
1324+
return;
13041325

13051326
task_work_add(current, &current->mce_kill_me, TWA_RESUME);
13061327
}
@@ -1438,7 +1459,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
14381459
/* If this triggers there is no way to recover. Die hard. */
14391460
BUG_ON(!on_thread_stack() || !user_mode(regs));
14401461

1441-
queue_task_work(&m, kill_current_task);
1462+
queue_task_work(&m, msg, kill_current_task);
14421463

14431464
} else {
14441465
/*
@@ -1456,7 +1477,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
14561477
}
14571478

14581479
if (m.kflags & MCE_IN_KERNEL_COPYIN)
1459-
queue_task_work(&m, kill_current_task);
1480+
queue_task_work(&m, msg, kill_current_task);
14601481
}
14611482
out:
14621483
mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);

include/linux/sched.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,7 @@ struct task_struct {
14681468
mce_whole_page : 1,
14691469
__mce_reserved : 62;
14701470
struct callback_head mce_kill_me;
1471+
int mce_count;
14711472
#endif
14721473

14731474
#ifdef CONFIG_KRETPROBES

0 commit comments

Comments
 (0)