During binding, check the status when calling callback functions
In a separate bug, we encounter a selinux denial when the update_engine
domain tries to call function from gmscore domain (through the callback).
This results in an infinite progress bar in the UI, and adds confusion
to testers and users.
Even though the callback function is defined as oneway in the aidl, I find
it returns status with the error message upon selinux denial:
Status(-129, EX_TRANSACTION_FAILED): 'FAILED_TRANSACTION: '
So, we should at least check this status and return false in the bind().
We also need to unbind the callback in such case. So the caller can
bind() again later without storing two callbacks in update_engine.
Bug: 146073270
Bug: 145340049
Test: check the error message and return value upon selinux denial
Change-Id: I7aeb80637704907090974192deaa17ba3eadc822
diff --git a/binder_service_android.cc b/binder_service_android.cc
index 9d040d6..994dcfa 100644
--- a/binder_service_android.cc
+++ b/binder_service_android.cc
@@ -70,6 +70,20 @@
Status BinderUpdateEngineAndroidService::bind(
const android::sp<IUpdateEngineCallback>& callback, bool* return_value) {
+ // Send an status update on connection (except when no update sent so far).
+ // Even though the status update is oneway, it still returns an erroneous
+ // status in case of a selinux denial. We should at least check this status
+ // and fails the binding.
+ if (last_status_ != -1) {
+ auto status = callback->onStatusUpdate(last_status_, last_progress_);
+ if (!status.isOk()) {
+ LOG(ERROR) << "Failed to call onStatusUpdate() from callback: "
+ << status.toString8();
+ *return_value = false;
+ return Status::ok();
+ }
+ }
+
callbacks_.emplace_back(callback);
const android::sp<IBinder>& callback_binder =
@@ -82,12 +96,6 @@
base::Unretained(this),
base::Unretained(callback_binder.get())));
- // Send an status update on connection (except when no update sent so far),
- // since the status update is oneway and we don't need to wait for the
- // response.
- if (last_status_ != -1)
- callback->onStatusUpdate(last_status_, last_progress_);
-
*return_value = true;
return Status::ok();
}