commit e6106102f8abf2887201fbfa9ba700ce521daa3f
author Danil Chapovalov <danilchap@webrtc.org> Wed Feb 16 11:29:02 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Feb 16 12:37:35 2022
tree c3c713b4f1116d8f070d3d38abf246baf4b27af6
parent 44fd6e35d3d375aed51b718b11cec0fe9ad00667

Fix fd leak in ifaddrs_android.cc

allow absl::Cleanup for such purpose

Bug: webrtc:13674
Change-Id: I7434c7a48f1135bf4bf14b66996fbff1a7016c74
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251781
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36016}

DEPS

diff --git a/DEPS b/DEPS
index c86b9c5..9bc9237 100644
--- a/DEPS
+++ b/DEPS
@@ -2675,6 +2675,7 @@
   "+absl/base/config.h",
   "+absl/base/const_init.h",
   "+absl/base/macros.h",
+  "+absl/cleanup/cleanup.h",
   "+absl/container/inlined_vector.h",
   "+absl/functional/bind_front.h",
   "+absl/memory/memory.h",

g3doc/abseil-in-webrtc.md

diff --git a/g3doc/abseil-in-webrtc.md b/g3doc/abseil-in-webrtc.md
index 9f7b4ab..6f0b481 100644
--- a/g3doc/abseil-in-webrtc.md
+++ b/g3doc/abseil-in-webrtc.md
@@ -26,6 +26,7 @@
 ## **Allowed**
 
 * `absl::bind_front`
+* `absl::Cleanup`
 * `absl::InlinedVector`
 * `absl::WrapUnique`
 * `absl::optional` and related stuff from `absl/types/optional.h`.

rtc_base/BUILD.gn

diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 33d867a..0cac2a4 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -285,7 +285,7 @@
 }
 
 config("chromium_logging_config") {
-  defines = ["LOGGING_INSIDE_WEBRTC"]
+  defines = [ "LOGGING_INSIDE_WEBRTC" ]
 }
 
 rtc_library("logging") {
@@ -312,12 +312,13 @@
       "../../webrtc_overrides/rtc_base/logging.cc",
       "../../webrtc_overrides/rtc_base/logging.h",
     ]
+
     # This macro needs to be both present in all WebRTC targets (see its
     # definition in //BUILD.gn but also propagated to all the targets
     # depending on the Chromium component defined in
     # //third_party/webrtc_overrides:webrtc_component. This public_config
     # allows GN to propagate the macro accordingly.
-    public_configs = [":chromium_logging_config"]
+    public_configs = [ ":chromium_logging_config" ]
   } else {
     sources = [
       "logging.cc",
@@ -887,6 +888,7 @@
       "log",
       "GLESv2",
     ]
+    absl_deps = [ "//third_party/abseil-cpp/absl/cleanup" ]
   }
 }
 

rtc_base/ifaddrs_android.cc

diff --git a/rtc_base/ifaddrs_android.cc b/rtc_base/ifaddrs_android.cc
index 1cc63fe..6474fb7 100644
--- a/rtc_base/ifaddrs_android.cc
+++ b/rtc_base/ifaddrs_android.cc
@@ -24,6 +24,8 @@
 #include <sys/utsname.h>
 #include <unistd.h>
 
+#include "absl/cleanup/cleanup.h"
+
 namespace {
 
 struct netlinkrequest {
@@ -138,10 +140,12 @@
 }
 
 int getifaddrs(struct ifaddrs** result) {
+  *result = nullptr;
   int fd = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
   if (fd < 0) {
     return -1;
   }
+  absl::Cleanup close_file = [fd] { close(fd); };
 
   netlinkrequest ifaddr_request;
   memset(&ifaddr_request, 0, sizeof(ifaddr_request));
@@ -151,10 +155,10 @@
 
   ssize_t count = send(fd, &ifaddr_request, ifaddr_request.header.nlmsg_len, 0);
   if (static_cast<size_t>(count) != ifaddr_request.header.nlmsg_len) {
-    close(fd);
     return -1;
   }
   struct ifaddrs* start = nullptr;
+  absl::Cleanup cleanup_start = [&start] { freeifaddrs(start); };
   struct ifaddrs* current = nullptr;
   char buf[kMaxReadSize];
   ssize_t amount_read = recv(fd, &buf, kMaxReadSize, 0);
@@ -165,13 +169,12 @@
          header = NLMSG_NEXT(header, header_size)) {
       switch (header->nlmsg_type) {
         case NLMSG_DONE:
-          // Success. Return.
+          // Success. Return `start`. Cancel `start` cleanup  because it
+          // becomes callers responsibility.
+          std::move(cleanup_start).Cancel();
           *result = start;
-          close(fd);
           return 0;
         case NLMSG_ERROR:
-          close(fd);
-          freeifaddrs(start);
           return -1;
         case RTM_NEWADDR: {
           ifaddrmsg* address_msg =
@@ -192,8 +195,6 @@
               }
               if (populate_ifaddrs(newest, address_msg, RTA_DATA(rta),
                                    RTA_PAYLOAD(rta)) != 0) {
-                freeifaddrs(start);
-                *result = nullptr;
                 return -1;
               }
               current = newest;
@@ -206,8 +207,6 @@
     }
     amount_read = recv(fd, &buf, kMaxReadSize, 0);
   }
-  close(fd);
-  freeifaddrs(start);
   return -1;
 }