Merge "Make Parcel::readOutVectorSizeWithCheck public"
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 0aca163..2c2a1b6 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -947,7 +947,10 @@
threadState->setCallingWorkSourceUidWithoutPropagation(workSource);
// vendor header
int32_t header = readInt32();
- if (header != kHeader) {
+
+ // fuzzers skip this check, because it is for protecting the underlying ABI, but
+ // we don't want it to reduce our coverage
+ if (header != kHeader && !mServiceFuzzing) {
ALOGE("Expecting header 0x%x but found 0x%x. Mixing copies of libbinder?", kHeader,
header);
return false;
@@ -966,10 +969,18 @@
(!len || !memcmp(parcel_interface, interface, len * sizeof (char16_t)))) {
return true;
} else {
- ALOGW("**** enforceInterface() expected '%s' but read '%s'",
- String8(interface, len).string(),
- String8(parcel_interface, parcel_interface_len).string());
- return false;
+ if (mServiceFuzzing) {
+ // ignore. Theoretically, this could cause a few false positives, because
+ // people could assume things about getInterfaceDescriptor if they pass
+ // this point, but it would be extremely fragile. It's more important that
+ // we fuzz with the above things read from the Parcel.
+ return true;
+ } else {
+ ALOGW("**** enforceInterface() expected '%s' but read '%s'",
+ String8(interface, len).string(),
+ String8(parcel_interface, parcel_interface_len).string());
+ return false;
+ }
}
}
@@ -977,6 +988,10 @@
mEnforceNoDataAvail = enforceNoDataAvail;
}
+void Parcel::setServiceFuzzing() {
+ mServiceFuzzing = true;
+}
+
binder::Status Parcel::enforceNoDataAvail() const {
if (!mEnforceNoDataAvail) {
return binder::Status::ok();
@@ -1722,7 +1737,9 @@
do {
if (mDataPos < kernelFields->mObjects[nextObject] + sizeof(flat_binder_object)) {
// Requested info overlaps with an object
- ALOGE("Attempt to read from protected data in Parcel %p", this);
+ if (!mServiceFuzzing) {
+ ALOGE("Attempt to read from protected data in Parcel %p", this);
+ }
return PERMISSION_DENIED;
}
nextObject++;
@@ -2092,7 +2109,11 @@
size_t len;
const char* str = readString8Inplace(&len);
if (str) return String8(str, len);
- ALOGE("Reading a NULL string not supported here.");
+
+ if (!mServiceFuzzing) {
+ ALOGE("Reading a NULL string not supported here.");
+ }
+
return String8();
}
@@ -2132,7 +2153,11 @@
size_t len;
const char16_t* str = readString16Inplace(&len);
if (str) return String16(str, len);
- ALOGE("Reading a NULL string not supported here.");
+
+ if (!mServiceFuzzing) {
+ ALOGE("Reading a NULL string not supported here.");
+ }
+
return String16();
}
@@ -2172,7 +2197,9 @@
{
status_t status = readNullableStrongBinder(val);
if (status == OK && !val->get()) {
- ALOGW("Expecting binder but got null!");
+ if (!mServiceFuzzing) {
+ ALOGW("Expecting binder but got null!");
+ }
status = UNEXPECTED_NULL;
}
return status;
@@ -2237,9 +2264,11 @@
if (const auto* rpcFields = maybeRpcFields()) {
if (!std::binary_search(rpcFields->mObjectPositions.begin(),
rpcFields->mObjectPositions.end(), mDataPos)) {
- ALOGW("Attempt to read file descriptor from Parcel %p at offset %zu that is not in the "
- "object list",
- this, mDataPos);
+ if (!mServiceFuzzing) {
+ ALOGW("Attempt to read file descriptor from Parcel %p at offset %zu that is not in "
+ "the object list",
+ this, mDataPos);
+ }
return BAD_TYPE;
}
@@ -2497,8 +2526,11 @@
return obj;
}
}
- ALOGW("Attempt to read object from Parcel %p at offset %zu that is not in the object list",
- this, DPOS);
+ if (!mServiceFuzzing) {
+ ALOGW("Attempt to read object from Parcel %p at offset %zu that is not in the object "
+ "list",
+ this, DPOS);
+ }
}
return nullptr;
}
@@ -3093,6 +3125,7 @@
mDeallocZero = false;
mOwner = nullptr;
mEnforceNoDataAvail = true;
+ mServiceFuzzing = false;
}
void Parcel::scanForFds() const {
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index eca084f..52cf1d1 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -149,6 +149,10 @@
// This Api is used by fuzzers to skip dataAvail checks.
void setEnforceNoDataAvail(bool enforceNoDataAvail);
+ // When fuzzing, we want to remove certain ABI checks that cause significant
+ // lost coverage, and we also want to avoid logs that cost too much to write.
+ void setServiceFuzzing();
+
void freeData();
size_t objectsCount() const;
@@ -1334,6 +1338,7 @@
// Set this to false to skip dataAvail checks.
bool mEnforceNoDataAvail;
+ bool mServiceFuzzing;
release_func mOwner;
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 41856f9..24fd2a6 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -804,12 +804,5 @@
cc_defaults {
name: "fuzzer_disable_leaks",
- fuzz_config: {
- asan_options: [
- "detect_leaks=0",
- ],
- hwasan_options: [
- "detect_leaks=0",
- ],
- },
+ //TODO(b/286112918) : Readd leak detection options
}
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index f88cfd6..1ff1de4 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -1125,7 +1125,7 @@
::testing::Values(true), ::testing::Values(true)),
BinderRpc::PrintParamInfo);
#else // BINDER_RPC_TO_TRUSTY_TEST
-static bool testSupportVsockLoopback() {
+bool testSupportVsockLoopback() {
// We don't need to enable TLS to know if vsock is supported.
unsigned int vsockPort = allocateVsockPort();
@@ -1225,7 +1225,15 @@
if (hasPreconnected) ret.push_back(SocketType::PRECONNECTED);
+#ifdef __BIONIC__
+ // Devices may not have vsock support. AVF tests will verify whether they do, but
+ // we can't require it due to old kernels for the time being.
static bool hasVsockLoopback = testSupportVsockLoopback();
+#else
+ // On host machines, we always assume we have vsock loopback. If we don't, the
+ // subsequent failures will be more clear than showing one now.
+ static bool hasVsockLoopback = true;
+#endif
if (hasVsockLoopback) {
ret.push_back(SocketType::VSOCK);
diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp
index cb09a7f..7435f30 100644
--- a/libs/binder/tests/binderRpcTestService.cpp
+++ b/libs/binder/tests/binderRpcTestService.cpp
@@ -139,7 +139,8 @@
CHECK_EQ(OK, server->setupRawSocketServer(std::move(socketFd)));
break;
case SocketType::VSOCK:
- CHECK_EQ(OK, server->setupVsockServer(VMADDR_CID_LOCAL, serverConfig.vsockPort));
+ CHECK_EQ(OK, server->setupVsockServer(VMADDR_CID_LOCAL, serverConfig.vsockPort))
+ << "Need `sudo modprobe vsock_loopback`?";
break;
case SocketType::INET: {
CHECK_EQ(OK, server->setupInetServer(kLocalInetAddress, 0, &outPort));
diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
index 0b3902d..24a9345 100644
--- a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
+++ b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp
@@ -52,7 +52,8 @@
uint32_t flags = provider.ConsumeIntegral<uint32_t>();
Parcel data;
// for increased fuzz coverage
- data.setEnforceNoDataAvail(provider.ConsumeBool());
+ data.setEnforceNoDataAvail(false);
+ data.setServiceFuzzing();
sp<IBinder> target = options.extraBinders.at(
provider.ConsumeIntegralInRange<size_t>(0,
@@ -73,7 +74,8 @@
Parcel reply;
// for increased fuzz coverage
- reply.setEnforceNoDataAvail(provider.ConsumeBool());
+ reply.setEnforceNoDataAvail(false);
+ reply.setServiceFuzzing();
(void)target->transact(code, data, &reply, flags);
// feed back in binders and fds that are returned from the service, so that