x86/iopl: Cure TIF_IO_BITMAP inconsistencies

commit 8b68e978718f14fdcb080c2a7791c52a0d09bc6d upstream.

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: ea5f1cd7ab49 ("x86/ioperm: Remove bitmap if all permissions dropped")
Reported-by: syzbot+e2b1803445d236442e54@syzkaller.appspotmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/87wmdceom2.ffs@tglx
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index e2fab3ceb09fb7da1d307e69af014b24ea65735f..9a101150376db7a6a8abfffe714578805cf54024 100644 (file)
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -33,8 +33,9 @@ void io_bitmap_share(struct task_struct *tsk)
        set_tsk_thread_flag(tsk, TIF_IO_BITMAP);
 }
 
-static void task_update_io_bitmap(struct task_struct *tsk)
+static void task_update_io_bitmap(void)
 {
+       struct task_struct *tsk = current;
        struct thread_struct *t = &tsk->thread;
 
        if (t->iopl_emul == 3 || t->io_bitmap) {
@@ -54,7 +55,12 @@ void io_bitmap_exit(struct task_struct *tsk)
        struct io_bitmap *iobm = tsk->thread.io_bitmap;
 
        tsk->thread.io_bitmap = NULL;
-       task_update_io_bitmap(tsk);
+       /*
+        * Don't touch the TSS when invoked on a failed fork(). TSS
+        * reflects the state of @current and not the state of @tsk.
+        */
+       if (tsk == current)
+               task_update_io_bitmap();
        if (iobm && refcount_dec_and_test(&iobm->refcnt))
                kfree(iobm);
 }
@@ -192,8 +198,7 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
        }
 
        t->iopl_emul = level;
-       task_update_io_bitmap(current);
-
+       task_update_io_bitmap();
        return 0;
 }
 

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 38c517a786f4b57e0e0b069f621ce9c7accf3e02..34e7c49b8057dbc3775e65a9a8638be6eb7d5d15 100644 (file)
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -143,6 +143,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, unsigned long arg,
        frame->ret_addr = (unsigned long) ret_from_fork;
        p->thread.sp = (unsigned long) fork_frame;
        p->thread.io_bitmap = NULL;
+       clear_tsk_thread_flag(p, TIF_IO_BITMAP);
        p->thread.iopl_warn = 0;
        memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
 
@@ -401,6 +402,11 @@ void native_tss_update_io_bitmap(void)
        } else {
                struct io_bitmap *iobm = t->io_bitmap;
 
+               if (WARN_ON_ONCE(!iobm)) {
+                       clear_thread_flag(TIF_IO_BITMAP);
+                       native_tss_invalidate_io_bitmap();
+               }
+
                /*
                 * Only copy bitmap data when the sequence number differs. The
                 * update time is accounted to the incoming task.