File CVE-2023-33953.patch of Package grpc.42944
From e261b7b803f7048040344ee2a70a25b8c77327f5 Mon Sep 17 00:00:00 2001
From: AJ Heller <hork@google.com>
Date: Tue, 30 Apr 2024 14:29:59 -0700
Subject: [PATCH 2/2] =?UTF-8?q?[backport][iomgr][EventEngine]=20Improve=20?=
=?UTF-8?q?server=20handling=20of=20file=20descri=E2=80=A6=20(#36489)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
…ptor exhaustion (#33656)
---
src/core/lib/iomgr/tcp_server_posix.cc | 64 +++++++++++----
src/core/lib/iomgr/tcp_server_utils_posix.h | 14 +++-
.../iomgr/tcp_server_utils_posix_common.cc | 80 ++++++++++++-------
3 files changed, 115 insertions(+), 43 deletions(-)
diff --git a/src/core/lib/iomgr/tcp_server_posix.cc b/src/core/lib/iomgr/tcp_server_posix.cc
index f8ac9f186b..b312246d5c 100644
--- a/src/core/lib/iomgr/tcp_server_posix.cc
+++ b/src/core/lib/iomgr/tcp_server_posix.cc
@@ -16,13 +16,17 @@
*
*/
-/* FIXME: "posix" files shouldn't be depending on _GNU_SOURCE */
+#include <grpc/support/port_platform.h>
+
+#include <utility>
+
+#include <grpc/support/atm.h>
+
+// FIXME: "posix" files shouldn't be depending on _GNU_SOURCE
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
-#include <grpc/support/port_platform.h>
-
#include "src/core/lib/iomgr/port.h"
#ifdef GRPC_POSIX_SOCKET_TCP_SERVER
@@ -37,6 +41,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include <grpc/byte_buffer.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
@@ -55,6 +60,19 @@
#include "src/core/lib/iomgr/tcp_server.h"
#include "src/core/lib/iomgr/tcp_server_utils_posix.h"
#include "src/core/lib/iomgr/unix_sockets_posix.h"
+#include "src/core/lib/transport/error_utils.h"
+
+#define GRPC_LOG_EVERY_N_SEC(n, severity, format, ...) \
+ do { \
+ static std::atomic<uint64_t> prev{0}; \
+ uint64_t now = gpr_now(GPR_CLOCK_MONOTONIC).tv_sec; \
+ if (prev == 0 || now - prev > (n)*1000) { \
+ prev = now; \
+ gpr_log(severity, format, __VA_ARGS__); \
+ } \
+ } while (0)
+
+static int64_t kRetryAcceptWaitTime = int64_t(GPR_MS_PER_SEC);
static grpc_error* tcp_server_create(grpc_closure* shutdown_complete,
const grpc_channel_args* args,
@@ -203,21 +221,37 @@ static void on_read(void* arg, grpc_error* err) {
if (fd < 0) {
if (errno == EINTR) {
continue;
- } else if (errno == EAGAIN || errno == ECONNABORTED ||
- errno == EWOULDBLOCK) {
+ }
+ // When the process runs out of fds, accept4() returns EMFILE. When this
+ // happens, the connection is left in the accept queue until either a
+ // read event triggers the on_read callback, or time has passed and the
+ // accept should be re-tried regardless. This callback is not cancelled,
+ // so a spurious wakeup may occur even when there's nothing to accept.
+ // This is not a performant code path, but if an fd limit has been
+ // reached, the system is likely in an unhappy state regardless.
+ if (errno == EMFILE) {
+ GRPC_LOG_EVERY_N_SEC(1, GPR_ERROR, "%s",
+ "File descriptor limit reached. Retrying.");
+ grpc_fd_notify_on_read(sp->emfd, &sp->read_closure);
+ if (gpr_atm_full_xchg(&sp->retry_timer_armed, true)) return;
+ grpc_timer_init(&sp->retry_timer,
+ grpc_core::ExecCtx::Get()->Now() + kRetryAcceptWaitTime,
+ &sp->retry_closure);
+ return;
+ }
+ if (errno == EAGAIN || errno == ECONNABORTED || errno == EWOULDBLOCK) {
grpc_fd_notify_on_read(sp->emfd, &sp->read_closure);
return;
+ }
+ gpr_mu_lock(&sp->server->mu);
+ if (!sp->server->shutdown_listeners) {
+ gpr_log(GPR_ERROR, "Failed accept4: %s", strerror(errno));
} else {
- gpr_mu_lock(&sp->server->mu);
- if (!sp->server->shutdown_listeners) {
- gpr_log(GPR_ERROR, "Failed accept4: %s", strerror(errno));
- } else {
- /* if we have shutdown listeners, accept4 could fail, and we
- needn't notify users */
- }
- gpr_mu_unlock(&sp->server->mu);
- goto error;
+ // if we have shutdown listeners, accept4 could fail, and we
+ // needn't notify users
}
+ gpr_mu_unlock(&sp->server->mu);
+ goto error;
}
/* For UNIX sockets, the accept call might not fill up the member sun_path
@@ -385,6 +419,7 @@ static grpc_error* clone_port(grpc_tcp_listener* listener, unsigned count) {
sp->port_index = listener->port_index;
sp->fd_index = listener->fd_index + count - i;
GPR_ASSERT(sp->emfd);
+ grpc_tcp_server_listener_initialize_retry_timer(sp);
while (listener->server->tail->next != nullptr) {
listener->server->tail = listener->server->tail->next;
}
@@ -559,6 +594,7 @@ static void tcp_server_shutdown_listeners(grpc_tcp_server* s) {
if (s->active_ports) {
grpc_tcp_listener* sp;
for (sp = s->head; sp; sp = sp->next) {
+ grpc_timer_cancel(&sp->retry_timer);
grpc_fd_shutdown(sp->emfd,
GRPC_ERROR_CREATE_FROM_STATIC_STRING("Server shutdown"));
}
diff --git a/src/core/lib/iomgr/tcp_server_utils_posix.h b/src/core/lib/iomgr/tcp_server_utils_posix.h
index 390d6d266a..1e7376298e 100644
--- a/src/core/lib/iomgr/tcp_server_utils_posix.h
+++ b/src/core/lib/iomgr/tcp_server_utils_posix.h
@@ -25,6 +25,7 @@
#include "src/core/lib/iomgr/resolve_address.h"
#include "src/core/lib/iomgr/socket_utils_posix.h"
#include "src/core/lib/iomgr/tcp_server.h"
+#include "src/core/lib/iomgr/timer.h"
/* one listening port */
typedef struct grpc_tcp_listener {
@@ -46,6 +47,11 @@ typedef struct grpc_tcp_listener {
identified while iterating through 'next'. */
struct grpc_tcp_listener* sibling;
int is_sibling;
+ // If an accept4() call fails, a timer is started to drain the accept queue in
+ // case no further connection attempts reach the gRPC server.
+ grpc_closure retry_closure;
+ grpc_timer retry_timer;
+ gpr_atm retry_timer_armed;
} grpc_tcp_listener;
/* the overall server */
@@ -122,4 +128,10 @@ grpc_error* grpc_tcp_server_prepare_socket(grpc_tcp_server*, int fd,
/* Ruturn true if the platform supports ifaddrs */
bool grpc_tcp_server_have_ifaddrs(void);
-#endif /* GRPC_CORE_LIB_IOMGR_TCP_SERVER_UTILS_POSIX_H */
+/* Initialize (but don't start) the timer and callback to retry accept4() on a
+ listening socket after file descriptors have been exhausted. This must be
+ called when creating a new listener. */
+void grpc_tcp_server_listener_initialize_retry_timer(
+ grpc_tcp_listener* listener);
+
+#endif /* GRPC_SRC_CORE_LIB_IOMGR_TCP_SERVER_UTILS_POSIX_H */
diff --git a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc
index ee1cd5c102..6ab64bed5b 100644
--- a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc
+++ b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc
@@ -18,6 +18,8 @@
#include <grpc/support/port_platform.h>
+#include <grpc/support/atm.h>
+
#include "src/core/lib/iomgr/port.h"
#ifdef GRPC_POSIX_SOCKET_TCP_SERVER_UTILS_COMMON
@@ -77,6 +79,24 @@ static int get_max_accept_queue_size(void) {
return s_max_accept_queue_size;
}
+static void listener_retry_timer_cb(void* arg, grpc_error* err) {
+ // Do nothing if cancelled.
+ if (err != GRPC_ERROR_NONE) return;
+ grpc_tcp_listener* listener = static_cast<grpc_tcp_listener*>(arg);
+ gpr_atm_no_barrier_store(&listener->retry_timer_armed, false);
+ if (!grpc_fd_is_shutdown(listener->emfd)) {
+ grpc_fd_set_readable(listener->emfd);
+ }
+}
+
+void grpc_tcp_server_listener_initialize_retry_timer(
+ grpc_tcp_listener* listener) {
+ gpr_atm_no_barrier_store(&listener->retry_timer_armed, false);
+ grpc_timer_init_unset(&listener->retry_timer);
+ GRPC_CLOSURE_INIT(&listener->retry_closure, listener_retry_timer_cb, listener,
+ grpc_schedule_on_exec_ctx);
+}
+
static grpc_error* add_socket_to_server(grpc_tcp_server* s, int fd,
const grpc_resolved_address* addr,
unsigned port_index, unsigned fd_index,
@@ -88,37 +108,41 @@ static grpc_error* add_socket_to_server(grpc_tcp_server* s, int fd,
grpc_error* err =
grpc_tcp_server_prepare_socket(s, fd, addr, s->so_reuseport, &port);
- if (err == GRPC_ERROR_NONE) {
- GPR_ASSERT(port > 0);
- grpc_sockaddr_to_string(&addr_str, addr, 1);
- gpr_asprintf(&name, "tcp-server-listener:%s", addr_str);
- gpr_mu_lock(&s->mu);
- s->nports++;
- GPR_ASSERT(!s->on_accept_cb && "must add ports before starting server");
- sp = static_cast<grpc_tcp_listener*>(gpr_malloc(sizeof(grpc_tcp_listener)));
- sp->next = nullptr;
- if (s->head == nullptr) {
- s->head = sp;
- } else {
- s->tail->next = sp;
- }
- s->tail = sp;
- sp->server = s;
- sp->fd = fd;
- sp->emfd = grpc_fd_create(fd, name, true);
- memcpy(&sp->addr, addr, sizeof(grpc_resolved_address));
- sp->port = port;
- sp->port_index = port_index;
- sp->fd_index = fd_index;
- sp->is_sibling = 0;
- sp->sibling = nullptr;
- GPR_ASSERT(sp->emfd);
- gpr_mu_unlock(&s->mu);
- gpr_free(addr_str);
- gpr_free(name);
+
+ if (err != GRPC_ERROR_NONE) return err;
+
+ GPR_ASSERT(port > 0);
+ grpc_sockaddr_to_string(&addr_str, addr, 1);
+ gpr_asprintf(&name, "tcp-server-listener:%s", addr_str);
+ gpr_mu_lock(&s->mu);
+ s->nports++;
+ sp = static_cast<grpc_tcp_listener*>(gpr_malloc(sizeof(grpc_tcp_listener)));
+ sp->next = nullptr;
+
+ if (s->head == nullptr) {
+ s->head = sp;
+ } else {
+ s->tail->next = sp;
}
+ s->tail = sp;
+ sp->server = s;
+ sp->fd = fd;
+ sp->emfd = grpc_fd_create(fd, name, true);
+ grpc_tcp_server_listener_initialize_retry_timer(sp);
+ memcpy(&sp->addr, addr, sizeof(grpc_resolved_address));
+ sp->port = port;
+ sp->port_index = port_index;
+ sp->fd_index = fd_index;
+ sp->is_sibling = 0;
+ sp->sibling = nullptr;
+ GPR_ASSERT(sp->emfd);
+ gpr_mu_unlock(&s->mu);
*listener = sp;
+
+ gpr_free(addr_str);
+ gpr_free(name);
+
return err;
}
--
2.53.0
