libbinder: remove gMaxFds from Parcel
libbinder uses getrlimit to get the maximum number of file
descriptors and uses that limit to validate Flattenables.
This syscall is specific to Linux and Mac OS, so this patch
removes it and the gMaxFds variable. Parcel operations for
Flattenables with too many fds should fail anyway when
duplicating the descriptors using fcntl. This adds a test
to binderLibTest for that case.
Bug: 224644083
Test: atest binderLibTest
Change-Id: I7e969180c165c7ebed74c8a97098eb2265e0305c
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 038ce38..e67dd7b 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -87,7 +87,8 @@
static std::atomic<size_t> gParcelGlobalAllocCount;
static std::atomic<size_t> gParcelGlobalAllocSize;
-static size_t gMaxFds = 0;
+// Maximum number of file descriptors per Parcel.
+constexpr size_t kMaxFds = 1024;
// Maximum size of a blob to transfer in-place.
static const size_t BLOB_INPLACE_LIMIT = 16 * 1024;
@@ -1416,7 +1417,7 @@
const size_t len = val.getFlattenedSize();
const size_t fd_count = val.getFdCount();
- if ((len > INT32_MAX) || (fd_count >= gMaxFds)) {
+ if ((len > INT32_MAX) || (fd_count > kMaxFds)) {
// don't accept size_t values which may have come from an
// inadvertent conversion from a negative int.
return BAD_VALUE;
@@ -2158,7 +2159,7 @@
const size_t len = this->readInt32();
const size_t fd_count = this->readInt32();
- if ((len > INT32_MAX) || (fd_count >= gMaxFds)) {
+ if ((len > INT32_MAX) || (fd_count > kMaxFds)) {
// don't accept size_t values which may have come from an
// inadvertent conversion from a negative int.
return BAD_VALUE;
@@ -2747,18 +2748,6 @@
mAllowFds = true;
mDeallocZero = false;
mOwner = nullptr;
-
- // racing multiple init leads only to multiple identical write
- if (gMaxFds == 0) {
- struct rlimit result;
- if (!getrlimit(RLIMIT_NOFILE, &result)) {
- gMaxFds = (size_t)result.rlim_cur;
- //ALOGI("parcel fd limit set to %zu", gMaxFds);
- } else {
- ALOGW("Unable to getrlimit: %s", strerror(errno));
- gMaxFds = 1024;
- }
- }
}
void Parcel::scanForFds() const {
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 3e90726..b3ecf4c 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -30,6 +30,7 @@
#include <android-base/properties.h>
#include <android-base/result-gmock.h>
#include <android-base/result.h>
+#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <binder/Binder.h>
@@ -1232,6 +1233,53 @@
EXPECT_THAT(server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr), StatusEq(OK));
}
+struct TooManyFdsFlattenable : Flattenable<TooManyFdsFlattenable> {
+ TooManyFdsFlattenable(size_t fdCount) : mFdCount(fdCount) {}
+
+ // Flattenable protocol
+ size_t getFlattenedSize() const {
+ // Return a valid non-zero size here so we don't get an unintended
+ // BAD_VALUE from Parcel::write
+ return 16;
+ }
+ size_t getFdCount() const { return mFdCount; }
+ status_t flatten(void *& /*buffer*/, size_t & /*size*/, int *&fds, size_t &count) const {
+ for (size_t i = 0; i < count; i++) {
+ fds[i] = STDIN_FILENO;
+ }
+ return NO_ERROR;
+ }
+ status_t unflatten(void const *& /*buffer*/, size_t & /*size*/, int const *& /*fds*/,
+ size_t & /*count*/) {
+ /* This doesn't get called */
+ return NO_ERROR;
+ }
+
+ size_t mFdCount;
+};
+
+TEST_F(BinderLibTest, TooManyFdsFlattenable) {
+ rlimit origNofile;
+ int ret = getrlimit(RLIMIT_NOFILE, &origNofile);
+ ASSERT_EQ(0, ret);
+
+ // Restore the original file limits when the test finishes
+ base::ScopeGuard guardUnguard([&]() { setrlimit(RLIMIT_NOFILE, &origNofile); });
+
+ rlimit testNofile = {1024, 1024};
+ ret = setrlimit(RLIMIT_NOFILE, &testNofile);
+ ASSERT_EQ(0, ret);
+
+ Parcel parcel;
+ // Try to write more file descriptors than supported by the OS
+ TooManyFdsFlattenable tooManyFds1(1024);
+ EXPECT_THAT(parcel.write(tooManyFds1), StatusEq(-EMFILE));
+
+ // Try to write more file descriptors than the internal limit
+ TooManyFdsFlattenable tooManyFds2(1025);
+ EXPECT_THAT(parcel.write(tooManyFds2), StatusEq(BAD_VALUE));
+}
+
TEST(ServiceNotifications, Unregister) {
auto sm = defaultServiceManager();
using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback;