From d8daf7077abb9290cf1603fc73e12db618f92b12 Mon Sep 17 00:00:00 2001
From: Alexander Warg <alexander.warg@kernkonzept.com>
Date: Wed, 5 Oct 2016 15:20:59 +0200
Subject: [PATCH] Fix invalid initialization in 'new'

C++ allows the compiler to elide write to objects that are later
initialized with a constructor. This may lead to the situation that the
`_quota` member of a thread is not correctly initialized. To fix this we
need to pass the correct quota object to each Thread constructor
instead.

Change-Id: Iac0ad2963b86f8393df6ad0c18adde386d9b1179
---
 src/kern/app_cpu_thread.cpp       |  7 ++++++-
 src/kern/arm/main.cpp             |  2 +-
 src/kern/arm/thread-arm.cpp       | 11 ++++++-----
 src/kern/ia32/32/main-ia32-32.cpp |  2 +-
 src/kern/ia32/64/main-ia32-64.cpp |  2 +-
 src/kern/ia32/thread-ia32.cpp     |  3 ++-
 src/kern/kernel_thread-std.cpp    |  4 ++--
 src/kern/kernel_thread.cpp        |  5 +++--
 src/kern/mips/main.cpp            |  2 +-
 src/kern/mips/thread-mips.cpp     |  5 +++--
 src/kern/ppc32/main.cpp           |  2 +-
 src/kern/ppc32/thread-ppc32.cpp   | 11 ++++++-----
 src/kern/sparc/main.cpp           |  2 +-
 src/kern/sparc/thread-sparc.cpp   |  5 +++--
 src/kern/thread.cpp               | 14 ++++++--------
 src/kern/thread_object.cpp        | 11 ++++++-----
 16 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/src/kern/app_cpu_thread.cpp b/src/kern/app_cpu_thread.cpp
index 8717355..57d2d9d 100644
--- a/src/kern/app_cpu_thread.cpp
+++ b/src/kern/app_cpu_thread.cpp
@@ -28,6 +28,11 @@ IMPLEMENTATION [mp]:
 #include "timer_tick.h"
 #include "spin_lock.h"
 
+PUBLIC explicit inline
+App_cpu_thread::App_cpu_thread(Ram_quota *q)
+: Kernel_thread(q)
+{}
+
 PUBLIC static
 Kernel_thread *
 App_cpu_thread::may_be_create(Cpu_number cpu, bool cpu_never_seen_before)
@@ -38,7 +43,7 @@ App_cpu_thread::may_be_create(Cpu_number cpu, bool cpu_never_seen_before)
       return static_cast<Kernel_thread *>(kernel_context(cpu));
     }
 
-  Kernel_thread *t = new (Ram_quota::root) App_cpu_thread;
+  Kernel_thread *t = new (Ram_quota::root) App_cpu_thread(Ram_quota::root);
   assert (t);
 
   t->set_home_cpu(cpu);
diff --git a/src/kern/arm/main.cpp b/src/kern/arm/main.cpp
index c44c154..379f300 100644
--- a/src/kern/arm/main.cpp
+++ b/src/kern/arm/main.cpp
@@ -82,7 +82,7 @@ kernel_main()
   //  pic_disable_all();
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
   assert(((Mword)kernel->init_stack() & 7) == 0);
diff --git a/src/kern/arm/thread-arm.cpp b/src/kern/arm/thread-arm.cpp
index 727da91..632621e 100644
--- a/src/kern/arm/thread-arm.cpp
+++ b/src/kern/arm/thread-arm.cpp
@@ -330,11 +330,12 @@ IMPLEMENTATION [arm]:
     @post state() != 0
  */
 IMPLEMENT
-Thread::Thread()
-  : Sender(0),	// select optimized version of constructor
-    _pager(Thread_ptr::Invalid),
-    _exc_handler(Thread_ptr::Invalid),
-    _del_observer(0)
+Thread::Thread(Ram_quota *q)
+: Sender(0),
+  _pager(Thread_ptr::Invalid),
+  _exc_handler(Thread_ptr::Invalid),
+  _quota(q),
+  _del_observer(0)
 {
   assert (state(false) == 0);
 
diff --git a/src/kern/ia32/32/main-ia32-32.cpp b/src/kern/ia32/32/main-ia32-32.cpp
index 9f0ce7f..c1a73a3 100644
--- a/src/kern/ia32/32/main-ia32-32.cpp
+++ b/src/kern/ia32/32/main-ia32-32.cpp
@@ -45,7 +45,7 @@ kernel_main(void)
   main_arch();
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   assert_opt (kernel);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
diff --git a/src/kern/ia32/64/main-ia32-64.cpp b/src/kern/ia32/64/main-ia32-64.cpp
index 4d7aebd..3857361 100644
--- a/src/kern/ia32/64/main-ia32-64.cpp
+++ b/src/kern/ia32/64/main-ia32-64.cpp
@@ -42,7 +42,7 @@ kernel_main(void)
   main_arch();
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
 
diff --git a/src/kern/ia32/thread-ia32.cpp b/src/kern/ia32/thread-ia32.cpp
index 6ee6534..47f8365 100644
--- a/src/kern/ia32/thread-ia32.cpp
+++ b/src/kern/ia32/thread-ia32.cpp
@@ -55,11 +55,12 @@ IMPLEMENTATION [ia32,amd64,ux]:
 Trap_state::Handler Thread::nested_trap_handler FIASCO_FASTCALL;
 
 IMPLEMENT
-Thread::Thread()
+Thread::Thread(Ram_quota *q)
 : Receiver(),
   Sender(0),   // select optimized version of constructor
   _pager(Thread_ptr::Invalid),
   _exc_handler(Thread_ptr::Invalid),
+  _quota(q),
   _del_observer(0)
 {
   assert (state(false) == 0);
diff --git a/src/kern/kernel_thread-std.cpp b/src/kern/kernel_thread-std.cpp
index 55370e6..fdf4451 100644
--- a/src/kern/kernel_thread-std.cpp
+++ b/src/kern/kernel_thread-std.cpp
@@ -73,7 +73,7 @@ Kernel_thread::init_workload()
 	check(map(o, sigma0, sigma0, c, 0));
     }
 
-  Thread_object *sigma0_thread = new (Ram_quota::root) Thread_object();
+  Thread_object *sigma0_thread = new (Ram_quota::root) Thread_object(Ram_quota::root);
 
   assert(sigma0_thread);
 
@@ -99,7 +99,7 @@ Kernel_thread::init_workload()
   // prevent deletion of this thing
   boot_task->inc_ref();
 
-  Thread_object *boot_thread = new (Ram_quota::root) Thread_object();
+  Thread_object *boot_thread = new (Ram_quota::root) Thread_object(Ram_quota::root);
 
   assert (boot_thread);
 
diff --git a/src/kern/kernel_thread.cpp b/src/kern/kernel_thread.cpp
index 8b4cfe5..348fde9 100644
--- a/src/kern/kernel_thread.cpp
+++ b/src/kern/kernel_thread.cpp
@@ -45,8 +45,9 @@ IMPLEMENTATION:
 #include "watchdog.h"
 
 
-PUBLIC
-Kernel_thread::Kernel_thread() : Thread_object(Thread::Kernel)
+PUBLIC explicit
+Kernel_thread::Kernel_thread(Ram_quota *q)
+: Thread_object(q, Thread::Kernel)
 {}
 
 PUBLIC inline
diff --git a/src/kern/mips/main.cpp b/src/kern/mips/main.cpp
index b78ed4f..163059e 100644
--- a/src/kern/mips/main.cpp
+++ b/src/kern/mips/main.cpp
@@ -78,7 +78,7 @@ extern "C" void kernel_main()
   set_exit_question(&exit_question);
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
   assert(((Mword)kernel->init_stack() & 7) == 0);
diff --git a/src/kern/mips/thread-mips.cpp b/src/kern/mips/thread-mips.cpp
index 34a5af3..f5cbc89 100644
--- a/src/kern/mips/thread-mips.cpp
+++ b/src/kern/mips/thread-mips.cpp
@@ -99,10 +99,11 @@ IMPLEMENT inline void  Thread::user_ip(Mword ip)  { regs()->ip(ip); }
     @post state() != 0
  */
 IMPLEMENT
-Thread::Thread()
-: Sender            (0),	// select optimized version of constructor
+Thread::Thread(Ram_quota *q)
+: Sender(0),
   _pager(Thread_ptr::Invalid),
   _exc_handler(Thread_ptr::Invalid),
+  _quota(q),
   _del_observer(0)
 {
 
diff --git a/src/kern/ppc32/main.cpp b/src/kern/ppc32/main.cpp
index 6bc721a..75db154 100644
--- a/src/kern/ppc32/main.cpp
+++ b/src/kern/ppc32/main.cpp
@@ -82,7 +82,7 @@ int main()
   //  pic_disable_all();
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
   //kdb_ke("init");
diff --git a/src/kern/ppc32/thread-ppc32.cpp b/src/kern/ppc32/thread-ppc32.cpp
index 82ea299..b35812b 100644
--- a/src/kern/ppc32/thread-ppc32.cpp
+++ b/src/kern/ppc32/thread-ppc32.cpp
@@ -239,11 +239,12 @@ IMPLEMENTATION [ppc32]:
     @post state() != 0
  */
 IMPLEMENT
-Thread::Thread()
-  : Sender            (0),	// select optimized version of constructor
-    _pager(Thread_ptr::Invalid),
-    _exc_handler(Thread_ptr::Invalid),
-    _del_observer(0)
+Thread::Thread(Ram_quota *q)
+: Sender(0),
+  _pager(Thread_ptr::Invalid),
+  _exc_handler(Thread_ptr::Invalid),
+  _quota(q),
+  _del_observer(0)
 {
 
   assert(state(false) == 0);
diff --git a/src/kern/sparc/main.cpp b/src/kern/sparc/main.cpp
index fe77fb8..17ff28d 100644
--- a/src/kern/sparc/main.cpp
+++ b/src/kern/sparc/main.cpp
@@ -81,7 +81,7 @@ int main()
   //  pic_disable_all();
 
   // create kernel thread
-  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread;
+  static Kernel_thread *kernel = new (Ram_quota::root) Kernel_thread(Ram_quota::root);
   Task *const ktask = Kernel_task::kernel_task();
   check(kernel->bind(ktask, User<Utcb>::Ptr(0)));
   //kdb_ke("init");
diff --git a/src/kern/sparc/thread-sparc.cpp b/src/kern/sparc/thread-sparc.cpp
index 7123e18..0b0f6ab 100644
--- a/src/kern/sparc/thread-sparc.cpp
+++ b/src/kern/sparc/thread-sparc.cpp
@@ -164,10 +164,11 @@ IMPLEMENTATION [sparc]:
     @post state() != 0
  */
 IMPLEMENT
-Thread::Thread()
-  : Sender            (0),	// select optimized version of constructor
+Thread::Thread(Ram_quota *q)
+  : Sender(0),
     _pager(Thread_ptr::Invalid),
     _exc_handler(Thread_ptr::Invalid),
+    _quota(q),
     _del_observer(0)
 {
 
diff --git a/src/kern/thread.cpp b/src/kern/thread.cpp
index 53a883d..89929ba 100644
--- a/src/kern/thread.cpp
+++ b/src/kern/thread.cpp
@@ -96,7 +96,7 @@ public:
    *
    * @post state() != 0.
    */
-  Thread();
+  explicit Thread(Ram_quota *);
 
   int handle_page_fault(Address pfa, Mword error, Mword pc,
                         Return_frame *regs);
@@ -139,7 +139,7 @@ public:
   bool arch_ext_vcpu_enabled();
 
 protected:
-  explicit Thread(Context_mode_kernel);
+  explicit Thread(Ram_quota *, Context_mode_kernel);
 
   // More ipc state
   Thread_ptr _pager;
@@ -198,10 +198,8 @@ Thread::operator new(size_t, Ram_quota *q) throw ()
 {
   void *t = Kmem_alloc::allocator()->q_unaligned_alloc(q, Thread::Size);
   if (t)
-    {
-      memset(t, 0, sizeof(Thread));
-      reinterpret_cast<Thread*>(t)->_quota = q;
-    }
+    memset(t, 0, sizeof(Thread));
+
   return t;
 }
 
@@ -272,8 +270,8 @@ Thread::unbind()
     @param id user-visible thread ID of the sender
  */
 IMPLEMENT inline
-Thread::Thread(Context_mode_kernel)
-  : Receiver(), Sender(), _del_observer(0), _magic(magic)
+Thread::Thread(Ram_quota *q, Context_mode_kernel)
+  : Receiver(), Sender(), _quota(q), _del_observer(0), _magic(magic)
 {
   inc_ref();
   _space.space(Kernel_task::kernel_task());
diff --git a/src/kern/thread_object.cpp b/src/kern/thread_object.cpp
index bafb6ad..0591542 100644
--- a/src/kern/thread_object.cpp
+++ b/src/kern/thread_object.cpp
@@ -68,11 +68,12 @@ Obj_cap::revalidate(Kobject_iface *o)
   return deref() == o;
 }
 
-PUBLIC
-Thread_object::Thread_object() : Thread() {}
+PUBLIC explicit
+Thread_object::Thread_object(Ram_quota *q) : Thread(q) {}
 
-PUBLIC
-Thread_object::Thread_object(Context_mode_kernel k) : Thread(k) {}
+PUBLIC explicit
+Thread_object::Thread_object(Ram_quota *q, Context_mode_kernel k)
+: Thread(q, k) {}
 
 PUBLIC virtual
 bool
@@ -693,7 +694,7 @@ thread_factory(Ram_quota *q, Space *,
                int *err)
 {
   *err = L4_err::ENomem;
-  return new (q) Thread_object();
+  return new (q) Thread_object(q);
 }
 
 static inline void __attribute__((constructor)) FIASCO_INIT
-- 
2.7.4

