From fadba1133cc4e67ce8dd9d9709edd76f9285c704 Mon Sep 17 00:00:00 2001 From: "Evgeny Grin (Karlson2k)" Date: Thu, 15 Apr 2021 18:58:58 +0300 Subject: fix #6768: do not use TCP-specific syscalls on UNIX domain sockets --- ChangeLog | 10 ++++++++ configure.ac | 2 +- src/microhttpd/daemon.c | 60 ++++++++++++++++++++++++++++++++++++++++------- src/microhttpd/internal.h | 13 +++++++++- src/microhttpd/mhd_send.c | 13 +++++++--- 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index a9318e91..a9e1556a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +Fri 16 Apr 2021 10:23:39 AM CEST +Thu 15 Apr 2021 18:56:00 MSK + Fixed configure '--enable-sanitizer' paramenter. + Stopped pushing of patrial responses when limited by system maximum size + for sendmsg(). -EG +Fri 16 Apr 2021 10:23:39 AM CEST + Detect if a socket is a UNIX domain socket and do not try to play + with TCP corking options in this case (avoids useless failed + syscalls). -CG + Web 14 Apr 2021 22:20:00 MSK Fixed: use sendmsg() in POSIX-compatible way, do not try to send more than IOV_MAX elements per single call. -EG diff --git a/configure.ac b/configure.ac index e6cc7214..e9c1e03b 100644 --- a/configure.ac +++ b/configure.ac @@ -117,7 +117,7 @@ AC_ARG_ENABLE([linker-hardening], AC_ARG_ENABLE([sanitizer], [AS_HELP_STRING([--enable-sanitizer], [enable Address Sanitizer and Undefined Behavior Sanitizer])], [AS_IF([test x$enableval = xyes],[ - LDFLAGS="$CFLAGS -fsanitize=address,undefined -fno-omit-frame-pointer" + CFLAGS="$CFLAGS -fsanitize=address,undefined -fno-omit-frame-pointer" ])]) diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c index 7f814ec0..d3b9958d 100644 --- a/src/microhttpd/daemon.c +++ b/src/microhttpd/daemon.c @@ -2376,6 +2376,7 @@ psk_gnutls_adapter (gnutls_session_t session, * @param non_blck indicate that socket in non-blocking mode * @param sk_spipe_supprs indicate that the @a client_socket has * set SIGPIPE suppression + * @param sk_is_unix true if this is a UNIX domain socket (AF_UNIX) * @return pointer to the connection on success, NULL if this daemon could * not handle the connection (i.e. malloc failed, etc). * The socket will be closed in case of error; 'errno' is @@ -2388,7 +2389,8 @@ new_connection_prepare_ (struct MHD_Daemon *daemon, socklen_t addrlen, bool external_add, bool non_blck, - bool sk_spipe_supprs) + bool sk_spipe_supprs, + bool sk_is_unix) { struct MHD_Connection *connection; int eno = 0; @@ -2490,6 +2492,7 @@ new_connection_prepare_ (struct MHD_Daemon *daemon, connection->addr_len = addrlen; connection->socket_fd = client_socket; connection->sk_nonblck = non_blck; + connection->is_unix = sk_is_unix; connection->sk_spipe_suppress = sk_spipe_supprs; connection->daemon = daemon; connection->last_activity = MHD_monotonic_sec_counter (); @@ -2860,6 +2863,7 @@ cleanup: * @param non_blck indicate that socket in non-blocking mode * @param sk_spipe_supprs indicate that the @a client_socket has * set SIGPIPE suppression + * @param sk_is_unix true if this is a UNIX domain socket (AF_UNIX) * @return #MHD_YES on success, #MHD_NO if this daemon could * not handle the connection (i.e. malloc failed, etc). * The socket will be closed in any case; 'errno' is @@ -2872,7 +2876,8 @@ internal_add_connection (struct MHD_Daemon *daemon, socklen_t addrlen, bool external_add, bool non_blck, - bool sk_spipe_supprs) + bool sk_spipe_supprs, + bool sk_is_unix) { struct MHD_Connection *connection; @@ -2912,9 +2917,13 @@ internal_add_connection (struct MHD_Daemon *daemon, return MHD_NO; } - connection = new_connection_prepare_ (daemon, client_socket, addr, addrlen, - external_add, non_blck, - sk_spipe_supprs); + connection = new_connection_prepare_ (daemon, + client_socket, + addr, addrlen, + external_add, + non_blck, + sk_spipe_supprs, + sk_is_unix); if (NULL == connection) return MHD_NO; @@ -3339,6 +3348,7 @@ MHD_add_connection (struct MHD_Daemon *daemon, { bool sk_nonbl; bool sk_spipe_supprs; + bool sk_is_unix = false; /* NOT thread safe with internal thread. TODO: fix thread safety. */ if ((0 == (daemon->options & MHD_USE_INTERNAL_POLLING_THREAD)) && @@ -3409,6 +3419,20 @@ MHD_add_connection (struct MHD_Daemon *daemon, _ ("Failed to set noninheritable mode on new client socket.\n")); #endif } +#ifdef SO_DOMAIN + { + int af; + socklen_t len = sizeof (af); + + if ( (0 == getsockopt (daemon->listen_fd, + SOL_SOCKET, + SO_DOMAIN, + &af, + &len)) && + (AF_UNIX == af) ) + sk_is_unix = true; + } +#endif #if defined(MHD_USE_POSIX_THREADS) || defined(MHD_USE_W32_THREADS) if (NULL != daemon->worker_pool) @@ -3428,7 +3452,8 @@ MHD_add_connection (struct MHD_Daemon *daemon, addrlen, true, sk_nonbl, - sk_spipe_supprs); + sk_spipe_supprs, + sk_is_unix); } /* all pools are at their connection limit, must refuse */ MHD_socket_close_chk_ (client_socket); @@ -3445,7 +3470,8 @@ MHD_add_connection (struct MHD_Daemon *daemon, addrlen, true, sk_nonbl, - sk_spipe_supprs); + sk_spipe_supprs, + sk_is_unix); } @@ -3627,7 +3653,8 @@ MHD_accept_connection (struct MHD_Daemon *daemon) addrlen, false, sk_nonbl, - sk_spipe_supprs); + sk_spipe_supprs, + daemon->listen_is_unix); return MHD_YES; } @@ -5810,8 +5837,24 @@ parse_options_va (struct MHD_Daemon *daemon, return MHD_NO; } else + { daemon->listen_fd = va_arg (ap, MHD_socket); +#ifdef SO_DOMAIN + { + int af; + socklen_t len = sizeof (af); + + if ( (0 == getsockopt (daemon->listen_fd, + SOL_SOCKET, + SO_DOMAIN, + &af, + &len)) && + (AF_UNIX == af) ) + daemon->listen_is_unix = true; + } +#endif + } break; case MHD_OPTION_EXTERNAL_LOGGER: #ifdef HAVE_MESSAGES @@ -6680,7 +6723,6 @@ MHD_start_daemon_va (unsigned int flags, } } daemon->listen_fd = listen_fd; - if (0 != (*pflags & MHD_USE_IPv6)) { #ifdef IPPROTO_IPV6 diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h index dc38bada..c939d3e2 100644 --- a/src/microhttpd/internal.h +++ b/src/microhttpd/internal.h @@ -993,6 +993,11 @@ struct MHD_Connection */ MHD_socket socket_fd; + /** + * true if #socket_fd is a UNIX domain socket, false (TCP) otherwise. + */ + bool is_unix; + /** * true if #socket_fd is non-blocking, false otherwise. */ @@ -1459,11 +1464,17 @@ struct MHD_Daemon int epoll_fd; /** - * true if the listen socket is in the 'epoll' set, + * true if the @e listen_fd socket is in the 'epoll' set, * false if not. */ bool listen_socket_in_epoll; + /** + * true if the @e listen_fd socket is a UNIX domain socket, + * false if not. + */ + bool listen_is_unix; + #ifdef UPGRADE_SUPPORT #ifdef HTTPS_SUPPORT /** diff --git a/src/microhttpd/mhd_send.c b/src/microhttpd/mhd_send.c index c237d469..3810e121 100644 --- a/src/microhttpd/mhd_send.c +++ b/src/microhttpd/mhd_send.c @@ -165,6 +165,7 @@ MHD_connection_set_nodelay_state_ (struct MHD_Connection *connection, * Set required cork state for connection socket * * The function automatically updates sk_corked state. + * * @param connection the connection to manipulate * @param cork_state the requested new state of socket * @return true if succeed, false if failed @@ -177,6 +178,8 @@ connection_set_cork_state_ (struct MHD_Connection *connection, const MHD_SCKT_OPT_BOOL_ on_val = 1; int err_code; + if (connection->is_unix) + return true; if (0 == setsockopt (connection->socket_fd, IPPROTO_TCP, MHD_TCP_CORK_NOPUSH, @@ -240,6 +243,8 @@ pre_send_setopt (struct MHD_Connection *connection, * Final piece is indicated by push_data == true. */ const bool buffer_data = (! push_data); + if (connection->is_unix) + return; /* The goal is to minimise the total number of additional sys-calls * before and after send(). * The following tricky (over-)complicated algorithm typically use zero, @@ -461,14 +466,14 @@ static bool zero_send_ (struct MHD_Connection *connection) { int dummy; + + if (connection->is_unix) + return; mhd_assert (_MHD_OFF == connection->sk_corked); mhd_assert (_MHD_ON == connection->sk_nodelay); - dummy = 0; /* Mute compiler and analyzer warnings */ - if (0 == MHD_send_ (connection->socket_fd, &dummy, 0)) return true; - #ifdef HAVE_MESSAGES MHD_DLOG (connection->daemon, _ ("Zero-send failed: %s\n"), @@ -499,6 +504,8 @@ post_send_setopt (struct MHD_Connection *connection, * Final piece is indicated by push_data == true. */ const bool buffer_data = (! push_data); + if (connection->is_unix) + return; if (buffer_data) return; /* Nothing to do after send(). */ -- cgit v1.2.3