releases/4.4.157/kthread-fix-use-after-free-if-kthread-fork-fails.patch

   1 From 4d6501dce079c1eb6bf0b1d8f528a5e81770109e Mon Sep 17 00:00:00 2001
   2 From: Vegard Nossum <vegard.nossum@oracle.com>
   3 Date: Tue, 9 May 2017 09:39:59 +0200
   4 Subject: kthread: Fix use-after-free if kthread fork fails
   5
   6 From: Vegard Nossum <vegard.nossum@oracle.com>
   7
   8 commit 4d6501dce079c1eb6bf0b1d8f528a5e81770109e upstream.
   9
  10 If a kthread forks (e.g. usermodehelper since commit 1da5c46fa965) but
  11 fails in copy_process() between calling dup_task_struct() and setting
  12 p->set_child_tid, then the value of p->set_child_tid will be inherited
  13 from the parent and get prematurely freed by free_kthread_struct().
  14
  15     kthread()
  16      - worker_thread()
  17         - process_one_work()
  18         |  - call_usermodehelper_exec_work()
  19         |     - kernel_thread()
  20         |        - _do_fork()
  21         |           - copy_process()
  22         |              - dup_task_struct()
  23         |                 - arch_dup_task_struct()
  24         |                    - tsk->set_child_tid = current->set_child_tid // implied
  25         |              - ...
  26         |              - goto bad_fork_*
  27         |              - ...
  28         |              - free_task(tsk)
  29         |                 - free_kthread_struct(tsk)
  30         |                    - kfree(tsk->set_child_tid)
  31         - ...
  32         - schedule()
  33            - __schedule()
  34               - wq_worker_sleeping()
  35                  - kthread_data(task)->flags // UAF
  36
  37 The problem started showing up with commit 1da5c46fa965 since it reused
  38 ->set_child_tid for the kthread worker data.
  39
  40 A better long-term solution might be to get rid of the ->set_child_tid
  41 abuse. The comment in set_kthread_struct() also looks slightly wrong.
  42
  43 Debugged-by: Jamie Iles <jamie.iles@oracle.com>
  44 Fixes: 1da5c46fa965 ("kthread: Make struct kthread kmalloc'ed")
  45 Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
  46 Acked-by: Oleg Nesterov <oleg@redhat.com>
  47 Cc: Peter Zijlstra <peterz@infradead.org>
  48 Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  49 Cc: Andy Lutomirski <luto@kernel.org>
  50 Cc: Frederic Weisbecker <fweisbec@gmail.com>
  51 Cc: Jamie Iles <jamie.iles@oracle.com>
  52 Cc: stable@vger.kernel.org
  53 Link: http://lkml.kernel.org/r/20170509073959.17858-1-vegard.nossum@oracle.com
  54 Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  55 Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
  56 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  57
  58 ---
  59  kernel/fork.c |   17 ++++++++++++-----
  60  1 file changed, 12 insertions(+), 5 deletions(-)
  61
  62 --- a/kernel/fork.c
  63 +++ b/kernel/fork.c
  64 @@ -1337,6 +1337,18 @@ static struct task_struct *copy_process(
  65         if (!p)
  66                 goto fork_out;
  67
  68 +       /*
  69 +        * This _must_ happen before we call free_task(), i.e. before we jump
  70 +        * to any of the bad_fork_* labels. This is to avoid freeing
  71 +        * p->set_child_tid which is (ab)used as a kthread's data pointer for
  72 +        * kernel threads (PF_KTHREAD).
  73 +        */
  74 +       p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
  75 +       /*
  76 +        * Clear TID on mm_release()?
  77 +        */
  78 +       p->clear_child_tid = (clone_flags & CLONE_CHILD_CLEARTID) ? child_tidptr : NULL;
  79 +
  80         ftrace_graph_init_task(p);
  81
  82         rt_mutex_init_task(p);
  83 @@ -1498,11 +1510,6 @@ static struct task_struct *copy_process(
  84                 }
  85         }
  86
  87 -       p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
  88 -       /*
  89 -        * Clear TID on mm_release()?
  90 -        */
  91 -       p->clear_child_tid = (clone_flags & CLONE_CHILD_CLEARTID) ? child_tidptr : NULL;
  92  #ifdef CONFIG_BLOCK
  93         p->plug = NULL;
  94  #endif