From 62e3e4bd1ac6dafcf88b43c9d1febee1a061e49d Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Tue, 12 Sep 2017 16:21:55 +0900 Subject: [PATCH] Tweaks to tetheroffload VTS: fixes and fd leaks Specifically: - always use hidl_handle::setTo(..., true) to force fd ownership (and thereby cleanup in hidl_handle destructor) - initialize config HAL multiple times to try to trigger fd leaking bugs Test: as follows - make vts -j30 BUILD_GOOGLE_VTS=true && \ vts-tradefed run commandAndExit vts \ --skip-all-system-status-check \ --primary-abi-only \ --skip-preconditions \ --module VtsHalTetherOffloadConfigV1_0Target -l INFO I/ResultReporter: Invocation finished in 48s. PASSED: 4, FAILED: 0, MODULES: 1 of 1 - make vts -j30 BUILD_GOOGLE_VTS=true && \ vts-tradefed run commandAndExit vts \ --skip-all-system-status-check \ --primary-abi-only \ --skip-preconditions \ --module VtsHalTetherOffloadControlV1_0Target -l INFO Bug: 29337859 Bug: 32163131 Bug: 64976634 Bug: 65270149 Bug: 65529504 Change-Id: Ib47b35a01092bb2052d241f7bba951bbf7691b66 --- ...tsHalTetheroffloadConfigV1_0TargetTest.cpp | 59 +++++++++++-------- ...sHalTetheroffloadControlV1_0TargetTest.cpp | 6 +- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/tetheroffload/config/1.0/vts/functional/VtsHalTetheroffloadConfigV1_0TargetTest.cpp b/tetheroffload/config/1.0/vts/functional/VtsHalTetheroffloadConfigV1_0TargetTest.cpp index fc61e1c1c0..2cbe479dc9 100644 --- a/tetheroffload/config/1.0/vts/functional/VtsHalTetheroffloadConfigV1_0TargetTest.cpp +++ b/tetheroffload/config/1.0/vts/functional/VtsHalTetheroffloadConfigV1_0TargetTest.cpp @@ -91,35 +91,42 @@ class OffloadConfigHidlTest : public testing::VtsHalHidlTargetTestBase { // Ensure handles can be set with correct socket options. TEST_F(OffloadConfigHidlTest, TestSetHandles) { - unique_fd fd1(netlinkSocket(kFd1Groups)); - if (fd1.get() < 0) { - ALOGE("Unable to create conntrack handles: %d/%s", errno, strerror(errno)); - FAIL(); - } - native_handle_t* const nativeHandle1 = native_handle_create(1, 0); - nativeHandle1->data[0] = fd1.release(); - const hidl_handle h1 = hidl_handle(nativeHandle1); + // Try multiple times in a row to see if it provokes file descriptor leaks. + for (int i = 0; i < 1024; i++) { + unique_fd fd1(netlinkSocket(kFd1Groups)); + if (fd1.get() < 0) { + ALOGE("Unable to create conntrack handles: %d/%s", errno, strerror(errno)); + FAIL(); + } + native_handle_t* const nativeHandle1 = native_handle_create(1, 0); + nativeHandle1->data[0] = fd1.release(); + hidl_handle h1; + h1.setTo(nativeHandle1, true); - unique_fd fd2(netlinkSocket(kFd2Groups)); - if (fd2.get() < 0) { - ALOGE("Unable to create conntrack handles: %d/%s", errno, strerror(errno)); - FAIL(); - } - native_handle_t* const nativeHandle2 = native_handle_create(1, 0); - nativeHandle2->data[0] = fd2.release(); - const hidl_handle h2 = hidl_handle(nativeHandle2); + unique_fd fd2(netlinkSocket(kFd2Groups)); + if (fd2.get() < 0) { + ALOGE("Unable to create conntrack handles: %d/%s", errno, strerror(errno)); + FAIL(); + } + native_handle_t* const nativeHandle2 = native_handle_create(1, 0); + nativeHandle2->data[0] = fd2.release(); + hidl_handle h2; + h2.setTo(nativeHandle2, true); - const Return ret = config->setHandles(h1, h2, ASSERT_TRUE_CALLBACK); - ASSERT_TRUE(ret.isOk()); + const Return ret = config->setHandles(h1, h2, ASSERT_TRUE_CALLBACK); + ASSERT_TRUE(ret.isOk()); + } } // Passing a handle without an associated file descriptor should return an error // (e.g. "Failed Input Checks"). Check that this occurs when both FDs are empty. TEST_F(OffloadConfigHidlTest, TestSetHandleNone) { native_handle_t* const nativeHandle1 = native_handle_create(0, 0); - const hidl_handle h1 = hidl_handle(nativeHandle1); + hidl_handle h1; + h1.setTo(nativeHandle1, true); native_handle_t* const nativeHandle2 = native_handle_create(0, 0); - const hidl_handle h2 = hidl_handle(nativeHandle2); + hidl_handle h2; + h2.setTo(nativeHandle2, true); const Return ret = config->setHandles(h1, h2, ASSERT_FALSE_CALLBACK); ASSERT_TRUE(ret.isOk()); @@ -135,10 +142,12 @@ TEST_F(OffloadConfigHidlTest, TestSetHandle1Only) { } native_handle_t* const nativeHandle1 = native_handle_create(1, 0); nativeHandle1->data[0] = fd1.release(); - const hidl_handle h1 = hidl_handle(nativeHandle1); + hidl_handle h1; + h1.setTo(nativeHandle1, true); native_handle_t* const nativeHandle2 = native_handle_create(0, 0); - const hidl_handle h2 = hidl_handle(nativeHandle2); + hidl_handle h2; + h2.setTo(nativeHandle2, true); const Return ret = config->setHandles(h1, h2, ASSERT_FALSE_CALLBACK); ASSERT_TRUE(ret.isOk()); @@ -148,7 +157,8 @@ TEST_F(OffloadConfigHidlTest, TestSetHandle1Only) { // (e.g. "Failed Input Checks"). Check that this occurs when FD1 is empty. TEST_F(OffloadConfigHidlTest, TestSetHandle2OnlyNotOk) { native_handle_t* const nativeHandle1 = native_handle_create(0, 0); - const hidl_handle h1 = hidl_handle(nativeHandle1); + hidl_handle h1; + h1.setTo(nativeHandle1, true); unique_fd fd2(netlinkSocket(kFd2Groups)); if (fd2.get() < 0) { @@ -157,7 +167,8 @@ TEST_F(OffloadConfigHidlTest, TestSetHandle2OnlyNotOk) { } native_handle_t* const nativeHandle2 = native_handle_create(1, 0); nativeHandle2->data[0] = fd2.release(); - const hidl_handle h2 = hidl_handle(nativeHandle2); + hidl_handle h2; + h2.setTo(nativeHandle2, true); const Return ret = config->setHandles(h1, h2, ASSERT_FALSE_CALLBACK); ASSERT_TRUE(ret.isOk()); diff --git a/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp b/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp index 3059eac0c2..a9424bc5aa 100644 --- a/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp +++ b/tetheroffload/control/1.0/vts/functional/VtsHalTetheroffloadControlV1_0TargetTest.cpp @@ -127,7 +127,8 @@ class OffloadControlHidlTestBase : public testing::VtsHalHidlTargetTestBase { } native_handle_t* const nativeHandle1 = native_handle_create(1, 0); nativeHandle1->data[0] = fd1.release(); - hidl_handle h1 = hidl_handle(nativeHandle1); + hidl_handle h1; + h1.setTo(nativeHandle1, true); unique_fd fd2(conntrackSocket(NFNLGRP_CONNTRACK_UPDATE | NFNLGRP_CONNTRACK_DESTROY)); if (fd2.get() < 0) { @@ -136,7 +137,8 @@ class OffloadControlHidlTestBase : public testing::VtsHalHidlTargetTestBase { } native_handle_t* const nativeHandle2 = native_handle_create(1, 0); nativeHandle2->data[0] = fd2.release(); - hidl_handle h2 = hidl_handle(nativeHandle2); + hidl_handle h2; + h2.setTo(nativeHandle2, true); const Return ret = config->setHandles(h1, h2, ASSERT_TRUE_CALLBACK); ASSERT_TRUE(ret.isOk());