From 382bafdd546d324a11541e3b68eb93b24f7bacbf Mon Sep 17 00:00:00 2001 From: Alexander Warg Date: Wed, 5 Oct 2016 15:20:59 +0200 Subject: [PATCH] FOC: 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 --- kernel/fiasco/src/kern/app_cpu_thread.cpp | 7 ++++++- kernel/fiasco/src/kern/arm/main.cpp | 2 +- kernel/fiasco/src/kern/arm/thread-arm.cpp | 11 ++++++----- kernel/fiasco/src/kern/ia32/32/main-ia32-32.cpp | 2 +- kernel/fiasco/src/kern/ia32/64/main-ia32-64.cpp | 2 +- kernel/fiasco/src/kern/ia32/thread-ia32.cpp | 3 ++- kernel/fiasco/src/kern/kernel_thread-std.cpp | 4 ++-- kernel/fiasco/src/kern/kernel_thread.cpp | 5 +++-- kernel/fiasco/src/kern/mips/main.cpp | 2 +- kernel/fiasco/src/kern/mips/thread-mips.cpp | 5 +++-- kernel/fiasco/src/kern/ppc32/main.cpp | 2 +- kernel/fiasco/src/kern/ppc32/thread-ppc32.cpp | 11 ++++++----- kernel/fiasco/src/kern/sparc/main.cpp | 2 +- kernel/fiasco/src/kern/sparc/thread-sparc.cpp | 5 +++-- kernel/fiasco/src/kern/thread.cpp | 14 ++++++-------- kernel/fiasco/src/kern/thread_object.cpp | 11 ++++++----- 16 files changed, 49 insertions(+), 39 deletions(-) diff --git a/kernel/fiasco/src/kern/app_cpu_thread.cpp b/kernel/fiasco/src/kern/app_cpu_thread.cpp index 8717355e..57d2d9d0 100644 --- a/kernel/fiasco/src/kern/app_cpu_thread.cpp +++ b/kernel/fiasco/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_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/kernel/fiasco/src/kern/arm/main.cpp b/kernel/fiasco/src/kern/arm/main.cpp index c44c154d..379f3002 100644 --- a/kernel/fiasco/src/kern/arm/main.cpp +++ b/kernel/fiasco/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::Ptr(0))); assert(((Mword)kernel->init_stack() & 7) == 0); diff --git a/kernel/fiasco/src/kern/arm/thread-arm.cpp b/kernel/fiasco/src/kern/arm/thread-arm.cpp index 727da91d..632621ec 100644 --- a/kernel/fiasco/src/kern/arm/thread-arm.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/ia32/32/main-ia32-32.cpp b/kernel/fiasco/src/kern/ia32/32/main-ia32-32.cpp index 9f0ce7fb..c1a73a36 100644 --- a/kernel/fiasco/src/kern/ia32/32/main-ia32-32.cpp +++ b/kernel/fiasco/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::Ptr(0))); diff --git a/kernel/fiasco/src/kern/ia32/64/main-ia32-64.cpp b/kernel/fiasco/src/kern/ia32/64/main-ia32-64.cpp index 4d7aebdb..3857361f 100644 --- a/kernel/fiasco/src/kern/ia32/64/main-ia32-64.cpp +++ b/kernel/fiasco/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::Ptr(0))); diff --git a/kernel/fiasco/src/kern/ia32/thread-ia32.cpp b/kernel/fiasco/src/kern/ia32/thread-ia32.cpp index 6ee65344..47f83653 100644 --- a/kernel/fiasco/src/kern/ia32/thread-ia32.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/kernel_thread-std.cpp b/kernel/fiasco/src/kern/kernel_thread-std.cpp index 55370e68..fdf4451a 100644 --- a/kernel/fiasco/src/kern/kernel_thread-std.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/kernel_thread.cpp b/kernel/fiasco/src/kern/kernel_thread.cpp index 8b4cfe5c..348fde9c 100644 --- a/kernel/fiasco/src/kern/kernel_thread.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/mips/main.cpp b/kernel/fiasco/src/kern/mips/main.cpp index b78ed4f4..163059e1 100644 --- a/kernel/fiasco/src/kern/mips/main.cpp +++ b/kernel/fiasco/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::Ptr(0))); assert(((Mword)kernel->init_stack() & 7) == 0); diff --git a/kernel/fiasco/src/kern/mips/thread-mips.cpp b/kernel/fiasco/src/kern/mips/thread-mips.cpp index 34a5af3c..f5cbc891 100644 --- a/kernel/fiasco/src/kern/mips/thread-mips.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/ppc32/main.cpp b/kernel/fiasco/src/kern/ppc32/main.cpp index 6bc721af..75db1541 100644 --- a/kernel/fiasco/src/kern/ppc32/main.cpp +++ b/kernel/fiasco/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::Ptr(0))); //kdb_ke("init"); diff --git a/kernel/fiasco/src/kern/ppc32/thread-ppc32.cpp b/kernel/fiasco/src/kern/ppc32/thread-ppc32.cpp index 82ea299a..b35812b3 100644 --- a/kernel/fiasco/src/kern/ppc32/thread-ppc32.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/sparc/main.cpp b/kernel/fiasco/src/kern/sparc/main.cpp index fe77fb8b..17ff28d0 100644 --- a/kernel/fiasco/src/kern/sparc/main.cpp +++ b/kernel/fiasco/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::Ptr(0))); //kdb_ke("init"); diff --git a/kernel/fiasco/src/kern/sparc/thread-sparc.cpp b/kernel/fiasco/src/kern/sparc/thread-sparc.cpp index 7123e180..0b0f6ab9 100644 --- a/kernel/fiasco/src/kern/sparc/thread-sparc.cpp +++ b/kernel/fiasco/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/kernel/fiasco/src/kern/thread.cpp b/kernel/fiasco/src/kern/thread.cpp index 53a883d8..89929ba9 100644 --- a/kernel/fiasco/src/kern/thread.cpp +++ b/kernel/fiasco/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(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/kernel/fiasco/src/kern/thread_object.cpp b/kernel/fiasco/src/kern/thread_object.cpp index bafb6ad2..0591542a 100644 --- a/kernel/fiasco/src/kern/thread_object.cpp +++ b/kernel/fiasco/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