From f6ae4907239f436257f213dd6f28c320dae99e3f Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Thu, 25 Apr 2019 16:07:49 -0700 Subject: [PATCH] Fix data race in WebSocket where _url is accessed without protection in setThreadName Also fix with url usage + docker container uses fedora and works with tsan --- DOCKER_VERSION | 2 +- Dockerfile | 40 +++++++------------ ixwebsocket/IXWebSocket.cpp | 2 +- test/CMakeLists.txt | 3 ++ .../statsd-client-cpp/src/statsd_client.h | 2 +- ws/ixcobra/IXCobraConnection.cpp | 22 +++++----- ws/ixcobra/IXCobraConnection.h | 8 ++-- 7 files changed, 34 insertions(+), 45 deletions(-) diff --git a/DOCKER_VERSION b/DOCKER_VERSION index 9df886c4..428b770e 100644 --- a/DOCKER_VERSION +++ b/DOCKER_VERSION @@ -1 +1 @@ -1.4.2 +1.4.3 diff --git a/Dockerfile b/Dockerfile index d24abf9e..ac2f178e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,42 +1,32 @@ -# Build time -FROM debian:buster as build +FROM fedora:30 as build -ENV DEBIAN_FRONTEND noninteractive -RUN apt-get update -RUN apt-get -y install wget +RUN yum install -y gcc-g++ +RUN yum install -y cmake +RUN yum install -y make +RUN yum install -y openssl-devel + +RUN yum install -y wget RUN mkdir -p /tmp/cmake WORKDIR /tmp/cmake RUN wget https://github.com/Kitware/CMake/releases/download/v3.14.0/cmake-3.14.0-Linux-x86_64.tar.gz RUN tar zxf cmake-3.14.0-Linux-x86_64.tar.gz -RUN apt-get -y install g++ -RUN apt-get -y install libssl-dev -RUN apt-get -y install libz-dev -RUN apt-get -y install make - -COPY . . - ARG CMAKE_BIN_PATH=/tmp/cmake/cmake-3.14.0-Linux-x86_64/bin ENV PATH="${CMAKE_BIN_PATH}:${PATH}" +RUN yum install -y python +RUN yum install -y libtsan + +COPY . . +# RUN ["make", "test"] RUN ["make"] # Runtime -FROM debian:buster as runtime +FROM fedora:30 as runtime -ENV DEBIAN_FRONTEND noninteractive -RUN apt-get update -# Runtime -RUN apt-get install -y libssl1.1 -RUN apt-get install -y ca-certificates -RUN ["update-ca-certificates"] +RUN yum install -y libtsan -# Debugging -RUN apt-get install -y strace -RUN apt-get install -y procps -RUN apt-get install -y htop - -RUN adduser --disabled-password --gecos '' app +RUN groupadd app && useradd -g app app COPY --chown=app:app --from=build /usr/local/bin/ws /usr/local/bin/ws RUN chmod +x /usr/local/bin/ws RUN ldd /usr/local/bin/ws diff --git a/ixwebsocket/IXWebSocket.cpp b/ixwebsocket/IXWebSocket.cpp index 34109feb..2c9d50b8 100644 --- a/ixwebsocket/IXWebSocket.cpp +++ b/ixwebsocket/IXWebSocket.cpp @@ -257,7 +257,7 @@ namespace ix void WebSocket::run() { - setThreadName(_url); + setThreadName(getUrl()); while (true) { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3a861a2d..8de914d6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -8,6 +8,9 @@ project (ixwebsocket_unittest) set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/../third_party/sanitizers-cmake/cmake" ${CMAKE_MODULE_PATH}) find_package(Sanitizers) +# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread") +# set(CMAKE_LD_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread") + set (CMAKE_CXX_STANDARD 14) if (NOT WIN32) diff --git a/third_party/statsd-client-cpp/src/statsd_client.h b/third_party/statsd-client-cpp/src/statsd_client.h index 2840a963..399ac4e3 100644 --- a/third_party/statsd-client-cpp/src/statsd_client.h +++ b/third_party/statsd-client-cpp/src/statsd_client.h @@ -61,6 +61,6 @@ protected: const uint64_t max_batching_size = 32768; }; -}; // end namespace +} // end namespace #endif diff --git a/ws/ixcobra/IXCobraConnection.cpp b/ws/ixcobra/IXCobraConnection.cpp index 072a7d55..dc5523c0 100644 --- a/ws/ixcobra/IXCobraConnection.cpp +++ b/ws/ixcobra/IXCobraConnection.cpp @@ -205,20 +205,18 @@ namespace ix } void CobraConnection::configure(const std::string& appkey, - const std::string& endpoint, - const std::string& rolename, - const std::string& rolesecret, - WebSocketPerMessageDeflateOptions webSocketPerMessageDeflateOptions) + const std::string& endpoint, + const std::string& rolename, + const std::string& rolesecret, + const WebSocketPerMessageDeflateOptions& webSocketPerMessageDeflateOptions) { - _appkey = appkey; - _endpoint = endpoint; - _role_name = rolename; - _role_secret = rolesecret; + _roleName = rolename; + _roleSecret = rolesecret; std::stringstream ss; - ss << _endpoint; + ss << endpoint; ss << "/v2?appkey="; - ss << _appkey; + ss << appkey; std::string url = ss.str(); _webSocket->setUrl(url); @@ -242,7 +240,7 @@ namespace ix bool CobraConnection::sendHandshakeMessage() { Json::Value data; - data["role"] = _role_name; + data["role"] = _roleName; Json::Value body; body["data"] = data; @@ -304,7 +302,7 @@ namespace ix bool CobraConnection::sendAuthMessage(const std::string& nonce) { Json::Value credentials; - credentials["hash"] = hmac(nonce, _role_secret); + credentials["hash"] = hmac(nonce, _roleSecret); Json::Value body; body["credentials"] = credentials; diff --git a/ws/ixcobra/IXCobraConnection.h b/ws/ixcobra/IXCobraConnection.h index 5f511267..fc2e8e7b 100644 --- a/ws/ixcobra/IXCobraConnection.h +++ b/ws/ixcobra/IXCobraConnection.h @@ -56,7 +56,7 @@ namespace ix const std::string& endpoint, const std::string& rolename, const std::string& rolesecret, - WebSocketPerMessageDeflateOptions webSocketPerMessageDeflateOptions); + const WebSocketPerMessageDeflateOptions& webSocketPerMessageDeflateOptions); static void setTrafficTrackerCallback(const TrafficTrackerCallback& callback); @@ -135,10 +135,8 @@ namespace ix std::unique_ptr _webSocket; /// Configuration data - std::string _appkey; - std::string _endpoint; - std::string _role_name; - std::string _role_secret; + std::string _roleName; + std::string _roleSecret; std::atomic _publishMode; // Can be set on control+background thread, protecting with an atomic