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

Commit 16d1aeec authored by Nandana Dutt's avatar Nandana Dutt
Browse files

dumpstate: enforce oneshot 

Ensure the service exits after errors as well as after successful
finish as a oneshot service should.

While at it also move remove(file) to unlink(file) and fix an error
in checking the return value of unlink.

BUG: 123571915
Test: adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test --gtest_filter=DumpstateBinderTest.*
Change-Id: I772b7981cd3b2f7c285ab980495d5539d57ebf46
parent afff99c5

cmds/dumpstate/DumpstateService.cpp

+38 −24
Original line number Diff line number Diff line
@@ -44,17 +44,18 @@ static binder::Status exception(uint32_t code, const std::string& msg) { 
    return binder::Status::fromExceptionCode(code, String8(msg.c_str()));

}



static binder::Status error(uint32_t code, const std::string& msg) {

    MYLOGE("%s (%d) ", msg.c_str(), code);

    return binder::Status::fromServiceSpecificError(code, String8(msg.c_str()));

}



// Takes ownership of data.

static void* callAndNotify(void* data) {

// Creates a bugreport and exits, thus preserving the oneshot nature of the service.

// Note: takes ownership of data.

[[noreturn]] static void* dumpstate_thread_main(void* data) {

    std::unique_ptr<DumpstateInfo> ds_info(static_cast<DumpstateInfo*>(data));

    ds_info->ds->Run(ds_info->calling_uid, ds_info->calling_package);

    MYLOGD("Finished Run()\n");

    return nullptr;

    MYLOGD("Finished taking a bugreport. Exiting.\n");

    exit(0);

}



[[noreturn]] static void signalErrorAndExit(sp<IDumpstateListener> listener, int error_code) {

    listener->onError(error_code);

    exit(0);

}



class DumpstateToken : public BnDumpstateToken {};
@@ -120,6 +121,25 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, 
                                                const sp<IDumpstateListener>& listener) {

    MYLOGI("startBugreport() with mode: %d\n", bugreport_mode);



    // This is the bugreporting API flow, so ensure there is only one bugreport in progress at a

    // time.

    std::lock_guard<std::mutex> lock(lock_);

    if (ds_ != nullptr) {

        MYLOGE("Error! There is already a bugreport in progress. Returning.");

        if (listener != nullptr) {

            listener->onError(IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);

        }

        return exception(binder::Status::EX_SERVICE_SPECIFIC,

                         "There is already a bugreport in progress");

    }



    // From here on, all conditions that indicate we are done with this incoming request should

    // result in exiting the service to free it up for next invocation.

    if (listener == nullptr) {

        MYLOGE("Invalid input: no listener");

        exit(0);

    }



    if (bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_FULL &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_REMOTE &&
@@ -127,30 +147,23 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, 
        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_TELEPHONY &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_WIFI &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_DEFAULT) {

        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,

                         StringPrintf("Invalid bugreport mode: %d", bugreport_mode));

        MYLOGE("Invalid input: bad bugreport mode: %d", bugreport_mode);

        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);

    }



    if (bugreport_fd.get() == -1 || screenshot_fd.get() == -1) {

        return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Invalid file descriptor");

        // TODO(b/111441001): screenshot fd should be optional

        MYLOGE("Invalid filedescriptor");

        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);

    }



    std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>();

    options->Initialize(static_cast<Dumpstate::BugreportMode>(bugreport_mode), bugreport_fd,

                        screenshot_fd);



    // This is the bugreporting API flow, so ensure there is only one bugreport in progress at a

    // time.

    std::lock_guard<std::mutex> lock(lock_);

    if (ds_ != nullptr) {

        return exception(binder::Status::EX_SERVICE_SPECIFIC,

                         "There is already a bugreport in progress");

    }

    ds_ = &(Dumpstate::GetInstance());

    ds_->SetOptions(std::move(options));

    if (listener != nullptr) {

    ds_->listener_ = listener;

    }



    DumpstateInfo* ds_info = new DumpstateInfo();

    ds_info->ds = ds_;
@@ -158,11 +171,12 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, 
    ds_info->calling_package = calling_package;



    pthread_t thread;

    status_t err = pthread_create(&thread, nullptr, callAndNotify, ds_info);

    status_t err = pthread_create(&thread, nullptr, dumpstate_thread_main, ds_info);

    if (err != 0) {

        delete ds_info;

        ds_info = nullptr;

        return error(err, "Could not create a background thread.");

        MYLOGE("Could not create a thread");

        signalErrorAndExit(listener, IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);

    }

    return binder::Status::ok();

}

cmds/dumpstate/dumpstate.cpp

+19 −18
Original line number Diff line number Diff line
@@ -157,7 +157,7 @@ bool CopyFile(int in_fd, int out_fd) { 
}



static bool CopyFileToFd(const std::string& input_file, int out_fd) {

    MYLOGD("Going to copy bugreport file (%s) to %d\n", ds.path_.c_str(), out_fd);

    MYLOGD("Going to copy file (%s) to %d\n", input_file.c_str(), out_fd);



    // Obtain a handle to the source file.

    android::base::unique_fd in_fd(OpenForRead(input_file));
@@ -165,14 +165,14 @@ static bool CopyFileToFd(const std::string& input_file, int out_fd) { 
        if (CopyFile(in_fd.get(), out_fd)) {

            return true;

        }

        MYLOGE("Failed to copy zip file: %s\n", strerror(errno));

        MYLOGE("Failed to copy file: %s\n", strerror(errno));

    }

    return false;

}



static bool UnlinkAndLogOnError(const std::string& file) {

    if (unlink(file.c_str()) != -1) {

        MYLOGE("Failed to remove file (%s): %s\n", file.c_str(), strerror(errno));

    if (unlink(file.c_str())) {

        MYLOGE("Failed to unlink file (%s): %s\n", file.c_str(), strerror(errno));

        return false;

    }

    return true;
@@ -460,9 +460,7 @@ static bool dump_anrd_trace() { 
            if (!ds.AddZipEntry("anrd_trace.txt", path)) {

                MYLOGE("Unable to add anrd_trace file %s to zip file\n", path);

            } else {

                if (remove(path)) {

                    MYLOGE("Error removing anrd_trace file %s: %s", path, strerror(errno));

                }

                android::os::UnlinkAndLogOnError(path);

                return true;

            }

        } else {
@@ -1582,13 +1580,8 @@ void Dumpstate::DumpstateBoard() { 
    for (int i = 0; i < NUM_OF_DUMPS; i++) {

        paths.emplace_back(StringPrintf("%s/%s", ds.bugreport_internal_dir_.c_str(),

                                        kDumpstateBoardFiles[i].c_str()));

        remover.emplace_back(android::base::make_scope_guard(std::bind(

            [](std::string path) {

                if (remove(path.c_str()) != 0 && errno != ENOENT) {

                    MYLOGE("Could not remove(%s): %s\n", path.c_str(), strerror(errno));

                }

            },

            paths[i])));

        remover.emplace_back(android::base::make_scope_guard(

            std::bind([](std::string path) { android::os::UnlinkAndLogOnError(path); }, paths[i])));

    }



    sp<IDumpstateDevice> dumpstate_device(IDumpstateDevice::getService());
@@ -1749,9 +1742,7 @@ bool Dumpstate::FinishZipFile() { 
    ds.zip_file.reset(nullptr);



    MYLOGD("Removing temporary file %s\n", tmp_path_.c_str())

    if (remove(tmp_path_.c_str()) != 0) {

        MYLOGE("Failed to remove temporary file (%s): %s\n", tmp_path_.c_str(), strerror(errno));

    }

    android::os::UnlinkAndLogOnError(tmp_path_);



    return true;

}
@@ -2335,6 +2326,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, 


    register_sig_handler();



    // TODO(b/111441001): maybe skip if already started?

    if (options_->do_start_service) {

        MYLOGI("Starting 'dumpstate' service\n");

        android::status_t ret;
@@ -2481,15 +2473,24 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, 
            // Do an early return if there were errors. We make an exception for consent

            // timing out because it's possible the user got distracted. In this case the

            // bugreport is not shared but made available for manual retrieval.

            MYLOGI("User denied consent. Returning\n");

            return status;

        }

        if (options_->screenshot_fd.get() != -1) {

        if (options_->do_fb && options_->screenshot_fd.get() != -1) {

            bool copy_succeeded = android::os::CopyFileToFd(screenshot_path_,

                                                            options_->screenshot_fd.get());

            if (copy_succeeded) {

                android::os::UnlinkAndLogOnError(screenshot_path_);

            }

        }

        if (status == Dumpstate::RunStatus::USER_CONSENT_TIMED_OUT) {

            MYLOGI(

                "Did not receive user consent yet."

                " Will not copy the bugreport artifacts to caller.\n");

            // TODO(b/111441001):

            // 1. cancel outstanding requests

            // 2. check for result more frequently

        }

    }



    /* vibrate a few but shortly times to let user know it's finished */

cmds/dumpstate/tests/dumpstate_smoke_test.cpp

+204 −11
Original line number Diff line number Diff line
@@ -21,6 +21,10 @@ 
#include <libgen.h>



#include <android-base/file.h>

#include <android/os/BnDumpstate.h>

#include <android/os/BnDumpstateListener.h>

#include <binder/IServiceManager.h>

#include <binder/ProcessState.h>

#include <cutils/properties.h>

#include <ziparchive/zip_archive.h>
@@ -34,6 +38,24 @@ namespace dumpstate { 


using ::testing::Test;

using ::std::literals::chrono_literals::operator""s;

using android::base::unique_fd;



class DumpstateListener;



namespace {



sp<IDumpstate> GetDumpstateService() {

    return android::interface_cast<IDumpstate>(

        android::defaultServiceManager()->getService(String16("dumpstate")));

}



int OpenForWrite(const std::string& filename) {

    return TEMP_FAILURE_RETRY(open(filename.c_str(),

                                   O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,

                                   S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));

}



}  // namespace



struct SectionInfo {

    std::string name;
@@ -46,41 +68,71 @@ struct SectionInfo { 
 * Listens to bugreport progress and updates the user by writing the progress to STDOUT. All the

 * section details generated by dumpstate are added to a vector to be used by Tests later.

 */

class DumpstateListener : public IDumpstateListener {

class DumpstateListener : public BnDumpstateListener {

  public:

    int outFd_, max_progress_;

    std::shared_ptr<std::vector<SectionInfo>> sections_;

    DumpstateListener(int fd, std::shared_ptr<std::vector<SectionInfo>> sections)

        : outFd_(fd), max_progress_(5000), sections_(sections) {

        : out_fd_(fd), sections_(sections) {

    }



    DumpstateListener(int fd) : out_fd_(fd) {

    }



    binder::Status onProgress(int32_t progress) override {

        dprintf(outFd_, "\rIn progress %d", progress);

        dprintf(out_fd_, "\rIn progress %d", progress);

        return binder::Status::ok();

    }



    binder::Status onError(int32_t error_code) override {

        dprintf(outFd_, "\rError %d", error_code);

        std::lock_guard<std::mutex> lock(lock_);

        error_code_ = error_code;

        dprintf(out_fd_, "\rError code %d", error_code);

        return binder::Status::ok();

    }



    binder::Status onFinished() override {

        dprintf(outFd_, "\rFinished");

        std::lock_guard<std::mutex> lock(lock_);

        is_finished_ = true;

        dprintf(out_fd_, "\rFinished");

        return binder::Status::ok();

    }



    binder::Status onProgressUpdated(int32_t progress) override {

        dprintf(outFd_, "\rIn progress %d/%d", progress, max_progress_);

        dprintf(out_fd_, "\rIn progress %d/%d", progress, max_progress_);

        return binder::Status::ok();

    }



    binder::Status onMaxProgressUpdated(int32_t max_progress) override {

        std::lock_guard<std::mutex> lock(lock_);

        max_progress_ = max_progress;

        return binder::Status::ok();

    }



    binder::Status onSectionComplete(const ::std::string& name, int32_t status, int32_t size_bytes,

                                     int32_t duration_ms) override {

        std::lock_guard<std::mutex> lock(lock_);

        if (sections_.get() != nullptr) {

            sections_->push_back({name, status, size_bytes, duration_ms});

        }

        return binder::Status::ok();

    }

    IBinder* onAsBinder() override {

        return nullptr;



    bool getIsFinished() {

        std::lock_guard<std::mutex> lock(lock_);

        return is_finished_;

    }



    int getErrorCode() {

        std::lock_guard<std::mutex> lock(lock_);

        return error_code_;

    }



  private:

    int out_fd_;

    int max_progress_ = 5000;

    int error_code_ = -1;

    bool is_finished_ = false;

    std::shared_ptr<std::vector<SectionInfo>> sections_;

    std::mutex lock_;

};



/**
@@ -293,6 +345,147 @@ TEST_F(BugreportSectionTest, WifiSectionGenerated) { 
    SectionExists("DUMPSYS - wifi", /* bytes= */ 100000);

}



class DumpstateBinderTest : public Test {

  protected:

    void SetUp() override {

        // In case there is a stray service, stop it first.

        property_set("ctl.stop", "bugreportd");

        // dry_run results in a faster bugreport.

        property_set("dumpstate.dry_run", "true");

        // We need to receive some async calls later. Ensure we have binder threads.

        ProcessState::self()->startThreadPool();

    }



    void TearDown() override {

        property_set("ctl.stop", "bugreportd");

        property_set("dumpstate.dry_run", "");



        unlink("/data/local/tmp/tmp.zip");

        unlink("/data/local/tmp/tmp.png");

    }



    // Waits until listener gets the callbacks.

    void WaitTillExecutionComplete(DumpstateListener* listener) {

        // Wait till one of finished, error or timeout.

        static const int kBugreportTimeoutSeconds = 120;

        int i = 0;

        while (!listener->getIsFinished() && listener->getErrorCode() == -1 &&

               i < kBugreportTimeoutSeconds) {

            sleep(1);

            i++;

        }

    }

};



TEST_F(DumpstateBinderTest, Baseline) {

    // In the beginning dumpstate binder service is not running.

    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());

    EXPECT_EQ(ds_binder, nullptr);



    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service

    // and makes it wait.

    property_set("dumpstate.dry_run", "true");

    property_set("ctl.start", "bugreportd");



    // Now we are able to retrieve dumpstate binder service.

    ds_binder = GetDumpstateService();

    EXPECT_NE(ds_binder, nullptr);



    // Prepare arguments

    unique_fd bugreport_fd(OpenForWrite("/bugreports/tmp.zip"));

    unique_fd screenshot_fd(OpenForWrite("/bugreports/tmp.png"));



    EXPECT_NE(bugreport_fd.get(), -1);

    EXPECT_NE(screenshot_fd.get(), -1);



    sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));

    android::binder::Status status =

        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,

                                  Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener);

    // startBugreport is an async call. Verify binder call succeeded first, then wait till listener

    // gets expected callbacks.

    EXPECT_TRUE(status.isOk());

    WaitTillExecutionComplete(listener.get());



    // Bugreport generation requires user consent, which we cannot get in a test set up,

    // so instead of getting is_finished_, we are more likely to get a consent error.

    EXPECT_TRUE(

        listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||

        listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);



    // The service should have died on its own, freeing itself up for a new invocation.

    sleep(2);

    ds_binder = GetDumpstateService();

    EXPECT_EQ(ds_binder, nullptr);

}



TEST_F(DumpstateBinderTest, ServiceDies_OnInvalidInput) {

    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service

    // and makes it wait.

    property_set("ctl.start", "bugreportd");

    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());

    EXPECT_NE(ds_binder, nullptr);



    // Prepare arguments

    unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));

    unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));



    EXPECT_NE(bugreport_fd.get(), -1);

    EXPECT_NE(screenshot_fd.get(), -1);



    // Call startBugreport with bad arguments.

    sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));

    android::binder::Status status =

        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,

                                  2000,  // invalid bugreport mode

                                  listener);

    EXPECT_EQ(listener->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);



    // The service should have died, freeing itself up for a new invocation.

    sleep(2);

    ds_binder = GetDumpstateService();

    EXPECT_EQ(ds_binder, nullptr);

}



TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) {

    // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service

    // and makes it wait.

    property_set("dumpstate.dry_run", "true");

    property_set("ctl.start", "bugreportd");

    sp<android::os::IDumpstate> ds_binder(GetDumpstateService());

    EXPECT_NE(ds_binder, nullptr);



    // Prepare arguments

    unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));

    unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));



    EXPECT_NE(bugreport_fd.get(), -1);

    EXPECT_NE(screenshot_fd.get(), -1);



    sp<DumpstateListener> listener1(new DumpstateListener(dup(fileno(stdout))));

    android::binder::Status status =

        ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,

                                  Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener1);

    EXPECT_TRUE(status.isOk());



    // try to make another call to startBugreport. This should fail.

    sp<DumpstateListener> listener2(new DumpstateListener(dup(fileno(stdout))));

    status = ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,

                                       Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2);

    EXPECT_FALSE(status.isOk());

    WaitTillExecutionComplete(listener2.get());

    EXPECT_EQ(listener2->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);



    // Meanwhile the first call works as expected. Service should not die in this case.

    WaitTillExecutionComplete(listener1.get());



    // Bugreport generation requires user consent, which we cannot get in a test set up,

    // so instead of getting is_finished_, we are more likely to get a consent error.

    EXPECT_TRUE(

        listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||

        listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);

}



}  // namespace dumpstate

}  // namespace os

}  // namespace android