From 96ee2a8e20bb7a7c4fb19e27dc31ff5c6a472849 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Mon, 06 Mar 2023 20:22:25 -0600 Subject: [PATCH] AddressTrackerLinux: Increase the message buffer size On non-4k-page systems, the message sizes may be too large to fit into the buffer, resulting in MSG_TRUNC. Instead of using the fixed 4kb size, follow the kernel documentation guidelines as to how large the buffer should be. Originally found by Asahi Lina: https://vt.social/@lina/109976892758680822 Bug: None Change-Id: I4790435190167a706fa7490ab57706db1f4a6120 --- diff --git a/net/base/address_tracker_linux.cc b/net/base/address_tracker_linux.cc index 4976cae..f1a1fff 100644 --- a/net/base/address_tracker_linux.cc +++ b/net/base/address_tracker_linux.cc @@ -14,6 +14,7 @@ #include "base/files/scoped_file.h" #include "base/functional/callback_helpers.h" #include "base/logging.h" +#include "base/memory/page_size.h" #include "base/posix/eintr_wrapper.h" #include "base/task/current_thread.h" #include "base/threading/scoped_blocking_call.h" @@ -323,8 +324,30 @@ *address_changed = false; *link_changed = false; *tunnel_changed = false; - char buffer[4096]; bool first_loop = true; + + // Varying sources have different opinions regarding the buffer size needed + // for netlink messages to avoid truncation: + // - The official documentation on netlink says messages are generally 8kb + // or the system page size, whichever is *larger*: + // https://www.kernel.org/doc/html/v6.2/userspace-api/netlink/intro.html#buffer-sizing + // - The kernel headers would imply that messages are generally the system + // page size or 8kb, whichever is *smaller*: + // https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/netlink.h?h=v6.2.2#n226 + // (libmnl follows this.) + // - The netlink(7) man page's example always uses a fixed size 8kb buffer: + // https://man7.org/linux/man-pages/man7/netlink.7.html + // Here, we follow the guidelines in the documentation, for two primary + // reasons: + // - Erring on the side of a larger size is the safer way to go to avoid + // MSG_TRUNC. + // - Since this is heap-allocated anyway, there's no risk to the stack by + // using the larger size. + + constexpr size_t kMinNetlinkBufferSize = 8 * 1024; + std::vector buffer( + std::max(base::GetPageSize(), kMinNetlinkBufferSize)); + { absl::optional blocking_call; if (tracking_) { @@ -334,9 +357,10 @@ } for (;;) { - int rv = HANDLE_EINTR(recv(netlink_fd_.get(), buffer, sizeof(buffer), - // Block the first time through loop. - first_loop ? 0 : MSG_DONTWAIT)); + int rv = + HANDLE_EINTR(recv(netlink_fd_.get(), buffer.data(), buffer.size(), + // Block the first time through loop. + first_loop ? 0 : MSG_DONTWAIT)); first_loop = false; if (rv == 0) { LOG(ERROR) << "Unexpected shutdown of NETLINK socket."; @@ -348,7 +372,8 @@ PLOG(ERROR) << "Failed to recv from netlink socket"; return; } - HandleMessage(buffer, rv, address_changed, link_changed, tunnel_changed); + HandleMessage(buffer.data(), rv, address_changed, link_changed, + tunnel_changed); } } if (*link_changed || *address_changed)