From 4832c7db4ba2fca72f10e0ed95d4edea63213451 Mon Sep 17 00:00:00 2001 From: Tomasz Sowa Date: Wed, 17 Feb 2021 15:58:05 +0100 Subject: [PATCH] fixed: clang address sanitizer reports that there is an use after free bug at the end when winix shuts down: ==3859==ERROR: AddressSanitizer: heap-use-after-free on address 0x61b0000053e0 at pc 0x000800697160 bp 0x7fffffffcfb0 sp 0x7fffffffcfa8 READ of size 8 at 0x61b0000053e0 thread T0 #0 0x80069715f in PT::MemBuffer::empty() const /usr/home/tomek/roboczy/prog/web.ttmath.org/../pikotools/membuffer/membuffer.h:675:9 #1 0x80069700d in PT::TextStreamBase::empty() const /usr/home/tomek/roboczy/prog/web.ttmath.org/../pikotools/textstream/textstream.h:161:16 #2 0x8009af0e3 in PT::FileLog::save_log(PT::TextStreamBase*) /usr/home/tomek/roboczy/prog/pikotools/log/filelog.cpp:105:14 #3 0x8009b3f8a in PT::Log::save_log() /usr/home/tomek/roboczy/prog/pikotools/log/log.cpp:461:13 #4 0x8009b3d72 in PT::Log::save_log_and_clear() /usr/home/tomek/roboczy/prog/pikotools/log/log.cpp:448:2 #5 0x8009b0627 in PT::Log::~Log() /usr/home/tomek/roboczy/prog/pikotools/log/log.cpp:62:2 #6 0x8007288e7 in Winix::Log::~Log() /usr/home/tomek/roboczy/prog/winix/winixd/core/log.cpp:56:1 #7 0x8007eac96 in Winix::WinixBase::~WinixBase() /usr/home/tomek/roboczy/prog/winix/winixd/core/winixbase.cpp:51:1 #8 0x8007eb097 in Winix::WinixModel::~WinixModel() /usr/home/tomek/roboczy/prog/winix/winixd/core/winixmodel.cpp:52:1 #9 0x80069e552 in Winix::BaseThread::~BaseThread() /usr/home/tomek/roboczy/prog/winix/winixd/core/basethread.cpp:54:1 #10 0x2f8132 in Winix::Job::~Job() /usr/home/tomek/roboczy/prog/web.ttmath.org/../winix/winixd/core/job.h:54:7 #11 0x2e0c8a in Winix::System::~System() /usr/home/tomek/roboczy/prog/web.ttmath.org/../winix/winixd/core/system.h:67:7 #12 0x2df017 in Winix::App::~App() /usr/home/tomek/roboczy/prog/web.ttmath.org/../winix/winixd/core/app.h:70:7 #13 0x800cc9f50 in __cxa_finalize /hddraidzfs/usr/src/lib/libc/stdlib/atexit.c:238:5 #14 0x800c5ef10 in exit /hddraidzfs/usr/src/lib/libc/stdlib/exit.c:74:2 #15 0x25c935 in _start /hddraidzfs/usr/src/lib/csu/amd64/crt1.c:76:2 0x61b0000053e0 is located 1632 bytes inside of 1656-byte region [0x61b000004d80,0x61b0000053f8) freed by thread T0 here: #0 0x2dc07d in operator delete(void*) /hddraidzfs/usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:160:3 #1 0x8007c3b2c in Winix::ThreadManager::StopAll() /usr/home/tomek/roboczy/prog/winix/winixd/core/threadmanager.cpp:189:3 #2 0x800647474 in Winix::App::WaitForThreads() /usr/home/tomek/roboczy/prog/winix/winixd/core/app.cpp:2122:24 #3 0x800647311 in Winix::App::Close() /usr/home/tomek/roboczy/prog/winix/winixd/core/app.cpp:340:2 #4 0x2dec7c in main /usr/home/tomek/roboczy/prog/winix/winixd/main/main.cpp:224:6 #5 0x25c92e in _start /hddraidzfs/usr/src/lib/csu/amd64/crt1.c:76:7 #6 0x80033efff () previously allocated by thread T0 here: #0 0x2db81d in operator new(unsigned long) /hddraidzfs/usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:99:3 #1 0x8007c17c3 in Winix::ThreadManager::Add(Winix::BaseThread*, wchar_t const*) /usr/home/tomek/roboczy/prog/winix/winixd/core/threadmanager.cpp:77:26 #2 0x8007b0b81 in Winix::System::Init() /usr/home/tomek/roboczy/prog/winix/winixd/core/system.cpp:170:17 #3 0x8006468d4 in Winix::App::Init() /usr/home/tomek/roboczy/prog/winix/winixd/core/app.cpp:295:9 #4 0x2deb7e in main /usr/home/tomek/roboczy/prog/winix/winixd/main/main.cpp:206:11 #5 0x25c92e in _start /hddraidzfs/usr/src/lib/csu/amd64/crt1.c:76:7 #6 0x80033efff () in ThreadManager::StopAll() there is item.thread_item_data pointer which was freed (the pointer was pointing to an object with a log_buffer) but internal Log buffer from item.object was pointing to that object and when thread_tab.clear() was called then the d-ctor was called and then save_log_and_clear() method was called and tried to use the freed object. --- winixd/core/basethread.cpp | 1 + winixd/core/threadmanager.cpp | 4 ++++ winixd/core/winixbase.cpp | 9 ++++++++- winixd/core/winixbase.h | 4 +++- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/winixd/core/basethread.cpp b/winixd/core/basethread.cpp index 151669d..a2d9a6e 100644 --- a/winixd/core/basethread.cpp +++ b/winixd/core/basethread.cpp @@ -254,6 +254,7 @@ void * BaseThread::StartRoutine(void * this_object) } } + base->save_log(); pthread_exit(0); return 0; } diff --git a/winixd/core/threadmanager.cpp b/winixd/core/threadmanager.cpp index 45087e1..d95f884 100644 --- a/winixd/core/threadmanager.cpp +++ b/winixd/core/threadmanager.cpp @@ -186,6 +186,10 @@ void ThreadManager::StopAll() item.object->WaitForThread(); log << log4 << "TM: thread " << id << " terminated" << logend; + // the thread is stopped and we can set the thread log buffer pointing to + // the main log buffer (from the main thread) + item.object->set_log_buffer(log.GetLogBuffer()); + delete item.thread_item_data; id += 1; } diff --git a/winixd/core/winixbase.cpp b/winixd/core/winixbase.cpp index 0cf9132..364532c 100644 --- a/winixd/core/winixbase.cpp +++ b/winixd/core/winixbase.cpp @@ -5,7 +5,7 @@ */ /* - * Copyright (c) 2018, Tomasz Sowa + * Copyright (c) 2018-2021, Tomasz Sowa * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -90,6 +90,13 @@ void WinixBase::set_dependency(WinixBase * winix_base) +void WinixBase::save_log() +{ + log << logsave; +} + + + } diff --git a/winixd/core/winixbase.h b/winixd/core/winixbase.h index 4bd66a3..2716df0 100644 --- a/winixd/core/winixbase.h +++ b/winixd/core/winixbase.h @@ -5,7 +5,7 @@ */ /* - * Copyright (c) 2018, Tomasz Sowa + * Copyright (c) 2018-2021, Tomasz Sowa * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -65,6 +65,8 @@ public: void set_dependency(WinixBase * winix_base); + void save_log(); + protected: