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

Commit 70197263 authored by Yifan Hong's avatar Yifan Hong
Browse files

health: Check return value of LinkedCallback::Make. 

If LinkedCallback::Make returns nullptr, do not put it
in Health::callback_. Otherwise, OnHealthInfoChanged
crashes later because the linked callback objects are
not null checked before accessing.

Test: android.hardware.health-service.aidl_fuzzer (with
  a special corpus)
Fixes: 289599278
Change-Id: I8bad41dbcfbefeb54744059baffd4eef1ae7ec42
parent 099240ae

health/aidl/default/Health.cpp

+5 −1
Original line number Diff line number Diff line
@@ -272,7 +272,11 @@ ndk::ScopedAStatus Health::registerCallback(const std::shared_ptr<IHealthInfoCal 


    {

        std::lock_guard<decltype(callbacks_lock_)> lock(callbacks_lock_);

        callbacks_.emplace_back(LinkedCallback::Make(ref<Health>(), callback));

        auto linked_callback_result = LinkedCallback::Make(ref<Health>(), callback);

        if (!linked_callback_result.ok()) {

            return ndk::ScopedAStatus::fromStatus(-linked_callback_result.error().code());

        }

        callbacks_.emplace_back(std::move(*linked_callback_result));

        // unlock

    }

health/aidl/default/LinkedCallback.cpp

+2 −2
Original line number Diff line number Diff line
@@ -24,7 +24,7 @@ 


namespace aidl::android::hardware::health {



std::unique_ptr<LinkedCallback> LinkedCallback::Make(

::android::base::Result<std::unique_ptr<LinkedCallback>> LinkedCallback::Make(

        std::shared_ptr<Health> service, std::shared_ptr<IHealthInfoCallback> callback) {

    std::unique_ptr<LinkedCallback> ret(new LinkedCallback());

    binder_status_t linkRet =
@@ -32,7 +32,7 @@ std::unique_ptr<LinkedCallback> LinkedCallback::Make( 
                                 reinterpret_cast<void*>(ret.get()));

    if (linkRet != ::STATUS_OK) {

        LOG(WARNING) << __func__ << "Cannot link to death: " << linkRet;

        return nullptr;

        return ::android::base::Error(-linkRet);

    }

    ret->service_ = service;

    ret->callback_ = std::move(callback);

health/aidl/default/LinkedCallback.h

+3 −2
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 


#include <aidl/android/hardware/health/IHealthInfoCallback.h>

#include <android-base/macros.h>

#include <android-base/result.h>

#include <android/binder_auto_utils.h>



#include <health-impl/Health.h>
@@ -34,8 +35,8 @@ class LinkedCallback { 
    // service->death_reciepient() should be from CreateDeathRecipient().

    // Not using a strong reference to |service| to avoid circular reference. The lifetime

    // of |service| must be longer than this LinkedCallback object.

    static std::unique_ptr<LinkedCallback> Make(std::shared_ptr<Health> service,

                                                std::shared_ptr<IHealthInfoCallback> callback);

    static ::android::base::Result<std::unique_ptr<LinkedCallback>> Make(

            std::shared_ptr<Health> service, std::shared_ptr<IHealthInfoCallback> callback);



    // Automatically unlinkToDeath upon destruction. So, it is always safe to reinterpret_cast

    // the cookie back to the LinkedCallback object.