Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a8186037 authored by sergiuferentz's avatar sergiuferentz Committed by Sergiu Ferentz
Browse files

Fix for heap-use-after-free in GPUService.cpp

This adds a unit test and fix for the bug reported by libfuzzer.
Changes made:
 * Expose GPUService as testable code.
 * Update main_gpuservice.cpp to use the new GpuService now located at
   gpuservice/GpuService.h
 * Make initializer threads members of GpuService
 * Join the threads in destructor to prevent heap-use-after-free.
 * Add unit test that waits 3 seconds after deallocation to ensure no
   wrong access is made.

Merged-In: I4d1d2d4658b575bf2c8f425f91f68f03114ad029
Bug: 282919145
Test: Added unit test and ran on device with ASAN
Change-Id: I4d1d2d4658b575bf2c8f425f91f68f03114ad029
(cherry picked from commit 3c00cbc0)
parent fe8e4d21
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -71,6 +71,7 @@ filegroup {
cc_library_shared {
    name: "libgpuservice",
    defaults: ["libgpuservice_production_defaults"],
    export_include_dirs: ["include"],
    srcs: [
        ":libgpuservice_sources",
    ],
+9 −5
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@

#define ATRACE_TAG ATRACE_TAG_GRAPHICS

#include "GpuService.h"
#include "gpuservice/GpuService.h"

#include <android-base/stringprintf.h>
#include <android-base/properties.h>
@@ -35,6 +35,7 @@
#include <vkjson.h>

#include <thread>
#include <memory>

namespace android {

@@ -58,18 +59,21 @@ GpuService::GpuService()
        mGpuStats(std::make_unique<GpuStats>()),
        mGpuMemTracer(std::make_unique<GpuMemTracer>()) {

    std::thread gpuMemAsyncInitThread([this]() {
    mGpuMemAsyncInitThread = std::make_unique<std::thread>([this] (){
        mGpuMem->initialize();
        mGpuMemTracer->initialize(mGpuMem);
    });
    gpuMemAsyncInitThread.detach();

    std::thread gpuWorkAsyncInitThread([this]() {
    mGpuWorkAsyncInitThread = std::make_unique<std::thread>([this]() {
        mGpuWork->initialize();
    });
    gpuWorkAsyncInitThread.detach();
};

GpuService::~GpuService() {
    mGpuWorkAsyncInitThread->join();
    mGpuMemAsyncInitThread->join();
}

void GpuService::setGpuStats(const std::string& driverPackageName,
                             const std::string& driverVersionName, uint64_t driverVersionCode,
                             int64_t driverBuildTime, const std::string& appPackageName,
+4 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@
#include <serviceutils/PriorityDumper.h>

#include <mutex>
#include <thread>
#include <vector>

namespace android {
@@ -41,6 +42,7 @@ public:
    static const char* const SERVICE_NAME ANDROID_API;

    GpuService() ANDROID_API;
    ~GpuService();

protected:
    status_t shellCommand(int in, int out, int err, std::vector<String16>& args) override;
@@ -90,6 +92,8 @@ private:
    std::unique_ptr<GpuMemTracer> mGpuMemTracer;
    std::mutex mLock;
    std::string mDeveloperDriverPath;
    std::unique_ptr<std::thread> mGpuMemAsyncInitThread;
    std::unique_ptr<std::thread> mGpuWorkAsyncInitThread;
};

} // namespace android
+1 −1
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@
#include <binder/IServiceManager.h>
#include <binder/ProcessState.h>
#include <sys/resource.h>
#include "GpuService.h"
#include "gpuservice/GpuService.h"

using namespace android;

+2 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ cc_test {
        "GpuMemTest.cpp",
        "GpuMemTracerTest.cpp",
        "GpuStatsTest.cpp",
        "GpuServiceTest.cpp",
    ],
    header_libs: ["bpf_headers"],
    shared_libs: [
@@ -45,6 +46,7 @@ cc_test {
        "libstatslog",
        "libstatspull",
        "libutils",
        "libgpuservice",
    ],
    static_libs: [
        "libgmock",
Loading