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}
This commit is contained in:
Danil Chapovalov 2022-02-16 12:29:02 +01:00 committed by WebRTC LUCI CQ
parent 44fd6e35d3
commit e6106102f8
4 changed files with 14 additions and 11 deletions

1
DEPS
View File

@ -2675,6 +2675,7 @@ include_rules = [
"+absl/base/config.h", "+absl/base/config.h",
"+absl/base/const_init.h", "+absl/base/const_init.h",
"+absl/base/macros.h", "+absl/base/macros.h",
"+absl/cleanup/cleanup.h",
"+absl/container/inlined_vector.h", "+absl/container/inlined_vector.h",
"+absl/functional/bind_front.h", "+absl/functional/bind_front.h",
"+absl/memory/memory.h", "+absl/memory/memory.h",

View File

@ -26,6 +26,7 @@ will generate a shared library.
## **Allowed** ## **Allowed**
* `absl::bind_front` * `absl::bind_front`
* `absl::Cleanup`
* `absl::InlinedVector` * `absl::InlinedVector`
* `absl::WrapUnique` * `absl::WrapUnique`
* `absl::optional` and related stuff from `absl/types/optional.h`. * `absl::optional` and related stuff from `absl/types/optional.h`.

View File

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

View File

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