Support deleting param for preconnected sessions
Although we can pass arbitrary pointers to setup preconnected clients,
the lifetime isn't clear and it's very easy to make UAF bugs.
To prevent UAF, an additional function can be passed to delete param
when param is no longer required, effectively transferring the ownership
of param to RPC session.
Bug: 398890208
Test: atest MicrodroidTests
Change-Id: I3bfb6d26567000c463b6e443734eadb29d945c06
diff --git a/android/vm_demo_native/main.cpp b/android/vm_demo_native/main.cpp
index e1acc05..8fc14bf 100644
--- a/android/vm_demo_native/main.cpp
+++ b/android/vm_demo_native/main.cpp
@@ -329,11 +329,15 @@
&ARpcSession_free);
ARpcSession_setMaxIncomingThreads(session.get(), 1);
+ auto param = std::make_unique<std::shared_ptr<IVirtualMachine>>(std::move(vm));
+ auto paramDeleteFd = [](void* param) {
+ delete static_cast<std::shared_ptr<IVirtualMachine>*>(param);
+ };
+
AIBinder* binder = ARpcSession_setupPreconnectedClient(
session.get(),
[](void* param) {
- std::shared_ptr<IVirtualMachine> vm =
- *static_cast<std::shared_ptr<IVirtualMachine>*>(param);
+ IVirtualMachine* vm = static_cast<std::shared_ptr<IVirtualMachine>*>(param)->get();
ScopedFileDescriptor sock_fd;
ScopedAStatus ret = vm->connectVsock(ITestService::PORT, &sock_fd);
if (!ret.isOk()) {
@@ -341,7 +345,7 @@
}
return sock_fd.release();
},
- &vm);
+ param.release(), paramDeleteFd);
if (binder == nullptr) {
return Error() << "Failed to connect to vm payload";
}
diff --git a/libs/libvirtualization_jni/android_system_virtualmachine_VirtualMachine.cpp b/libs/libvirtualization_jni/android_system_virtualmachine_VirtualMachine.cpp
index 67a4716..8452344 100644
--- a/libs/libvirtualization_jni/android_system_virtualmachine_VirtualMachine.cpp
+++ b/libs/libvirtualization_jni/android_system_virtualmachine_VirtualMachine.cpp
@@ -59,28 +59,29 @@
JNIEnv *mEnv;
jobject mProvider;
jmethodID mMid;
- } state;
+ };
- state.mEnv = env;
- state.mProvider = provider;
- state.mMid = mid;
+ auto state = std::make_unique<State>(env, provider, mid);
using RequestFun = int (*)(void *);
RequestFun requestFunc = [](void *param) -> int {
- State *state = reinterpret_cast<State *>(param);
+ State *state = static_cast<State *>(param);
int ownedFd = state->mEnv->CallIntMethod(state->mProvider, state->mMid);
// FD is owned by PFD in Java layer, need to dupe it so that
// ARpcSession_setupPreconnectedClient can take ownership when it calls unique_fd internally
return fcntl(ownedFd, F_DUPFD_CLOEXEC, 0);
};
+ auto paramDeleteFunc = [](void *param) { delete static_cast<State *>(param); };
+
RpcSessionHandle session;
// We need a thread pool to be able to support linkToDeath, or callbacks
// (b/268335700). These threads are currently created eagerly, so we don't
// want too many. The number 1 is chosen after some discussion, and to match
// the server-side default (mMaxThreads on RpcServer).
ARpcSession_setMaxIncomingThreads(session.get(), 1);
- auto client = ARpcSession_setupPreconnectedClient(session.get(), requestFunc, &state);
+ auto client = ARpcSession_setupPreconnectedClient(session.get(), requestFunc, state.release(),
+ paramDeleteFunc);
return AIBinder_toJavaBinder(env, client);
}