search

chriskohlhoff / asio

Asio C++ Library
http://think-async.com/Asio
4.85k stars 1.21k forks source link

Dereference NULL return value in ‘boost/asio/detail/impl/scheduler.ipp’ #847

Open dimitrov-ds opened 3 years ago

dimitrov-ds commented 3 years ago

Hi, I've just run Coverity, version 2020.03, static analysis on a program using Boost v 1_75_0 It has highlighted several potential issues one of which I will describe here.

The scheduler::compensating_work_started() method, on line 324, calls the thread¬_call_stack::contains() method and assigns the result to the local pointer variable this_thread. The thread_call_stack::contains() method can return NULL. However, on the next line 325, without a null check, the ‘this_thread’ pointer is dereferenced. If ‘this_pointer’ is null then dereferencing it will cause a segmentation fault. Note that there are number of examples in the same file where the result of the thread_call_stack::contains() is checked before dereferencing. Am I understanding the code correctly or I am missing something?

Thanks, Dimitar

wuxinmao2008 commented 2 years ago

I found the same problem on Raspberry Pi 3B, and this happens every time . But I don't know how it happens. image

wuxinmao2008 commented 2 years ago

Here is something about how it happen: I make a wrapper over asio::serial_port , using 2 backend thread to run asio::io_context . After opening the port, I call async_read from main thread (% version 1%). When data comes, then call async_read in read callback functions . All write is through this 

   asio::post(mStrand, [=](){
       asio::async_write(mSerialPort,asio::buffer(data->data(), data->size() ),
       [=](const asio::error_code& ec, std::size_t length){
             // ...
       });
    });

It worked fine on Windows & Ubuntu(x64). When I try to run this on Raspberry Pi 3B, it crash at position ( display in picture above).

vehlwn commented 1 year ago

+1. (Sorry for necroposting.) I was testing different security flags and found my code fails to compile with -Werror=null-dereference on gcc (GCC) 12.2.1 20230111, Arch Linux, system-wide boost 1.81.0-2. I'm attaching error logs along with minimal CMake example.

In function ‘bool boost::asio::operator==(const io_context::basic_executor_type<std::allocator<void>, 0>&, const io_context::basic_executor_type<std::allocator<void>, 0>&)’,
    inlined from ‘static bool boost::asio::execution::detail::any_executor_base::equal_ex(const boost::asio::execution::detail::any_executor_base&, const boost::asio::execution::detail::any_executor_base&) [with E
x = boost::asio::io_context::basic_executor_type<std::allocator<void>, 0>]’ at /usr/include/boost/asio/execution/any_executor.hpp:996:30:
/usr/include/boost/asio/io_context.hpp:994:14: error: potential null pointer dereference [-Werror=null-dereference]
  994 |     return a.target_ == b.target_
      |            ~~^~~~~~~
/usr/include/boost/asio/io_context.hpp:994:14: error: potential null pointer dereference [-Werror=null-dereference]
In file included from /usr/include/boost/asio/detail/scheduler.hpp:240,
                 from /usr/include/boost/asio/io_context.hpp:44:
In member function ‘void boost::asio::detail::scheduler::compensating_work_started()’,
    inlined from ‘boost::asio::detail::epoll_reactor::perform_io_cleanup_on_block_exit::~perform_io_cleanup_on_block_exit()’ at /usr/include/boost/asio/detail/impl/epoll_reactor.ipp:742:53,
    inlined from ‘boost::asio::detail::operation* boost::asio::detail::epoll_reactor::descriptor_state::perform_io(uint32_t)’ at /usr/include/boost/asio/detail/impl/epoll_reactor.ipp:794:1,
    inlined from ‘static void boost::asio::detail::epoll_reactor::descriptor_state::do_complete(void*, boost::asio::detail::operation*, const boost::system::error_code&, std::size_t)’ at /usr/include/boost/asio/de
tail/impl/epoll_reactor.ipp:804:52,
    inlined from ‘static void boost::asio::detail::epoll_reactor::descriptor_state::do_complete(void*, boost::asio::detail::operation*, const boost::system::error_code&, std::size_t)’ at /usr/include/boost/asio/de
tail/impl/epoll_reactor.ipp:796:6:
/usr/include/boost/asio/detail/impl/scheduler.ipp:331:45: error: potential null pointer dereference [-Werror=null-dereference]
  331 |   ++static_cast<thread_info*>(this_thread)->private_outstanding_work;
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

CMakeLists.txt:

Spoiler warning ```cmake cmake_minimum_required(VERSION 3.16) project(boost-asio-null-dereference LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(CMAKE_COLOR_DIAGNOSTICS ON) add_executable(a main.cpp) find_package(Boost REQUIRED COMPONENTS coroutine) target_include_directories(a SYSTEM PUBLIC "${Boost_INCLUDE_DIRS}") target_link_libraries(a PUBLIC "${Boost_LIBRARIES}") include(CheckCXXCompilerFlag) set(FLAG -Werror=null-dereference) check_cxx_compiler_flag(${FLAG} HAS_Wnull_dereference) if(HAS_Wnull_dereference) target_compile_options(a PRIVATE ${FLAG}) endif() include(CMakePrintHelpers) cmake_print_variables(Boost_LIBRARIES) cmake_print_variables(Boost_INCLUDE_DIRS) ```

main.cpp:

Spoiler warning ```c++ #include #include #include #include #include #include #include #include #include void process_request(boost::asio::io_context &io_context, boost::asio::ip::tcp::socket &&tcp_stream, boost::asio::yield_context yield) { constexpr std::string_view data = "HTTP/1.1 200 OK\r\n" "Content-Length: 2\r\n" "Content-Type: text/plain\r\n\r\n" "OK"; boost::asio::async_write(tcp_stream, boost::asio::buffer(data), boost::asio::transfer_all(), yield); } void main_coroutine(boost::asio::io_context &io_context, boost::asio::yield_context yield) { const auto host = boost::asio::ip::make_address("127.0.0.1"); const std::uint16_t port = 5000; const auto tcp_listen_addr = boost::asio::ip::tcp::endpoint{host, port}; auto tcp_listener = boost::asio::ip::tcp::acceptor{io_context}; tcp_listener.open(tcp_listen_addr.protocol()); tcp_listener.set_option(boost::asio::ip::tcp::acceptor::reuse_address(true)); tcp_listener.bind(tcp_listen_addr); tcp_listener.listen(); std::clog << "Listening on " << tcp_listener.local_endpoint() << std::endl; while (true) { boost::system::error_code ec; auto tcp_stream = boost::asio::ip::tcp::socket(io_context); tcp_listener.async_accept(tcp_stream, yield[ec]); if (ec) { std::cerr << "Error in tcp_listener.accept():" << ec.what(); continue; } std::clog << "Incoming connection from " << tcp_stream.remote_endpoint() << "/TCP" << std::endl; boost::asio::spawn( io_context, [&io_context, tcp_stream = std::move(tcp_stream)]( boost::asio::yield_context yield) mutable { try { process_request(io_context, std::move(tcp_stream), yield); } catch (const std::exception &ex) { std::cerr << "Error in process_request: " << ex.what(); } }); } } int main() { boost::asio::io_context io_context; boost::asio::spawn(io_context, [&](boost::asio::yield_context yield) { try { main_coroutine(io_context, yield); } catch (const std::exception &ex) { std::cerr << "main_coroutine: " << ex.what() << std::endl; } }); io_context.run(); } ```

Build:

$ cmake -GNinja .. -DCMAKE_BUILD_TYPE=Release
-- The CXX compiler identification is GNU 12.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Boost: /usr/lib/cmake/Boost-1.81.0/BoostConfig.cmake (found version "1.81.0") found components: coroutine
-- Performing Test HAS_Wnull_dereference
-- Performing Test HAS_Wnull_dereference - Success
-- Boost_LIBRARIES="Boost::coroutine"
-- Boost_INCLUDE_DIRS="/usr/include"
-- Configuring done
-- Generating done
-- Build files have been written to: ...
$ cmake --build .

I think this diagnostic is false positive because it also triggers on standard headers:

/usr/include/c++/12.2.1/bits/shared_ptr_base.h:1666:16: error: potential null pointer dereference [-Werror=null-dereference]
 1666 |       { return _M_ptr; }

PS: With clang it compiles without any warnings: CC=clang CXX=clang++ cmake -GNinja .. -DCMAKE_BUILD_TYPE=Release && cmake --build .

PPS: Does anyone know how to disable warnings from system headers? Because target_include_directories() function in CMake apparently don't work for CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES:

CMake does not explicitly specify these directories on compiler command lines for language . This prevents system include directories from being treated as user include directories on some compilers.

-- CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES="/usr/include/c++/12.2.1;/usr/include/c++/12.2.1/x86_64-pc-linux-gnu;/usr/include/c++/12.2.1/backward;/usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include;/usr/local/include;/u sr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include-fixed;/usr/include"

In compile_commands.json I have:

"command": "/usr/bin/c++ -DBOOST_CONTEXT_DYN_LINK -DBOOST_CONTEXT_NO_LIB -DBOOST_COROUTINE_DYN_LINK -DBOOST_COROUTINE_NO_LIB -O3 -DNDEBUG -fdiagnostics-color=always -Werror=null-dereference -std=c++17 -o CMakeFiles/a.dir/main.cpp.o -c /.../main.cpp",