Skip to content

Commit 8b68e97

Browse files
KAGA-KOKObp3tk0v
authored andcommitted
x86/iopl: Cure TIF_IO_BITMAP inconsistencies
io_bitmap_exit() is invoked from exit_thread() when a task exists or when a fork fails. In the latter case the exit_thread() cleans up resources which were allocated during fork(). io_bitmap_exit() invokes task_update_io_bitmap(), which in turn ends up in tss_update_io_bitmap(). tss_update_io_bitmap() operates on the current task. If current has TIF_IO_BITMAP set, but no bitmap installed, tss_update_io_bitmap() crashes with a NULL pointer dereference. There are two issues, which lead to that problem: 1) io_bitmap_exit() should not invoke task_update_io_bitmap() when the task, which is cleaned up, is not the current task. That's a clear indicator for a cleanup after a failed fork(). 2) A task should not have TIF_IO_BITMAP set and neither a bitmap installed nor IOPL emulation level 3 activated. This happens when a kernel thread is created in the context of a user space thread, which has TIF_IO_BITMAP set as the thread flags are copied and the IO bitmap pointer is cleared. Other than in the failed fork() case this has no impact because kernel threads including IO workers never return to user space and therefore never invoke tss_update_io_bitmap(). Cure this by adding the missing cleanups and checks: 1) Prevent io_bitmap_exit() to invoke task_update_io_bitmap() if the to be cleaned up task is not the current task. 2) Clear TIF_IO_BITMAP in copy_thread() unconditionally. For user space forks it is set later, when the IO bitmap is inherited in io_bitmap_share(). For paranoia sake, add a warning into tss_update_io_bitmap() to catch the case, when that code is invoked with inconsistent state. Fixes: ea5f1cd ("x86/ioperm: Remove bitmap if all permissions dropped") Reported-by: [email protected] Signed-off-by: Thomas Gleixner <[email protected]> Signed-off-by: Borislav Petkov (AMD) <[email protected]> Cc: [email protected] Link: https://lore.kernel.org/87wmdceom2.ffs@tglx
1 parent 99850a1 commit 8b68e97

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

arch/x86/kernel/ioport.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ void io_bitmap_share(struct task_struct *tsk)
3333
set_tsk_thread_flag(tsk, TIF_IO_BITMAP);
3434
}
3535

36-
static void task_update_io_bitmap(struct task_struct *tsk)
36+
static void task_update_io_bitmap(void)
3737
{
38+
struct task_struct *tsk = current;
3839
struct thread_struct *t = &tsk->thread;
3940

4041
if (t->iopl_emul == 3 || t->io_bitmap) {
@@ -54,7 +55,12 @@ void io_bitmap_exit(struct task_struct *tsk)
5455
struct io_bitmap *iobm = tsk->thread.io_bitmap;
5556

5657
tsk->thread.io_bitmap = NULL;
57-
task_update_io_bitmap(tsk);
58+
/*
59+
* Don't touch the TSS when invoked on a failed fork(). TSS
60+
* reflects the state of @current and not the state of @tsk.
61+
*/
62+
if (tsk == current)
63+
task_update_io_bitmap();
5864
if (iobm && refcount_dec_and_test(&iobm->refcnt))
5965
kfree(iobm);
6066
}
@@ -192,8 +198,7 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
192198
}
193199

194200
t->iopl_emul = level;
195-
task_update_io_bitmap(current);
196-
201+
task_update_io_bitmap();
197202
return 0;
198203
}
199204

arch/x86/kernel/process.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
176176
frame->ret_addr = (unsigned long) ret_from_fork_asm;
177177
p->thread.sp = (unsigned long) fork_frame;
178178
p->thread.io_bitmap = NULL;
179+
clear_tsk_thread_flag(p, TIF_IO_BITMAP);
179180
p->thread.iopl_warn = 0;
180181
memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
181182

@@ -464,6 +465,11 @@ void native_tss_update_io_bitmap(void)
464465
} else {
465466
struct io_bitmap *iobm = t->io_bitmap;
466467

468+
if (WARN_ON_ONCE(!iobm)) {
469+
clear_thread_flag(TIF_IO_BITMAP);
470+
native_tss_invalidate_io_bitmap();
471+
}
472+
467473
/*
468474
* Only copy bitmap data when the sequence number differs. The
469475
* update time is accounted to the incoming task.

0 commit comments

Comments
 (0)