From f9366e7bbd141300134f58ce622783ec567e34d1 Mon Sep 17 00:00:00 2001 From: John Salem Date: Fri, 16 Jul 2021 11:44:52 -0700 Subject: [PATCH 1/4] set CLOEXEC on diagnostic server FDs --- src/native/eventpipe/ds-ipc-pal-socket.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 328becb84693c7..7fd543158af651 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -316,8 +316,15 @@ ipc_socket_create_uds (DiagnosticsIpc *ipc) EP_ASSERT (ipc->server_address_family == AF_UNIX); ds_ipc_socket_t new_socket = DS_IPC_INVALID_SOCKET; + int socket_type = SOCK_STREAM; +#ifdef SOCK_CLOEXEC + socket_type |= SOCK_CLOEXEC; +#endif // SOCK_CLOEXEC DS_ENTER_BLOCKING_PAL_SECTION; - new_socket = socket (ipc->server_address_family, SOCK_STREAM, 0); + new_socket = socket (ipc->server_address_family, socket_type, 0); +#ifndef SOCK_CLOEXEC + fcntl(new_socket, F_SETFD, FD_CLOEXEC); +#endif // SOCK_CLOEXEC DS_EXIT_BLOCKING_PAL_SECTION; return new_socket; #endif @@ -337,8 +344,15 @@ ipc_socket_create_tcp (DiagnosticsIpc *ipc) #endif ds_ipc_socket_t new_socket = DS_IPC_INVALID_SOCKET; + int socket_type = SOCK_STREAM; +#ifdef SOCK_CLOEXEC + socket_type |= SOCK_CLOEXEC; +#endif // SOCK_CLOEXEC DS_ENTER_BLOCKING_PAL_SECTION; - new_socket = socket (ipc->server_address_family, SOCK_STREAM, IPPROTO_TCP); + new_socket = socket (ipc->server_address_family, socket_type, IPPROTO_TCP); +#ifndef SOCK_CLOEXEC + fcntl(new_socket, F_SETFD, FD_CLOEXEC); +#endif // SOCK_CLOEXEC if (new_socket != DS_IPC_INVALID_SOCKET) { int option_value = 1; setsockopt (new_socket, IPPROTO_TCP, TCP_NODELAY, (const char*)&option_value, sizeof (option_value)); From 90833acea8253a817c3d1c1c757b7df19c5f1b29 Mon Sep 17 00:00:00 2001 From: John Salem Date: Fri, 16 Jul 2021 12:45:36 -0700 Subject: [PATCH 2/4] Add assert and comment --- src/native/eventpipe/ds-ipc-pal-socket.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 7fd543158af651..1eac35d9601569 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -323,7 +323,8 @@ ipc_socket_create_uds (DiagnosticsIpc *ipc) DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, 0); #ifndef SOCK_CLOEXEC - fcntl(new_socket, F_SETFD, FD_CLOEXEC); + int fcntl_rc = fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value + EP_ASSERT (fcntl_rc != -1); #endif // SOCK_CLOEXEC DS_EXIT_BLOCKING_PAL_SECTION; return new_socket; @@ -351,7 +352,8 @@ ipc_socket_create_tcp (DiagnosticsIpc *ipc) DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, IPPROTO_TCP); #ifndef SOCK_CLOEXEC - fcntl(new_socket, F_SETFD, FD_CLOEXEC); + int fcntl_rc = fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value + EP_ASSERT(fcntl_rc != -1); #endif // SOCK_CLOEXEC if (new_socket != DS_IPC_INVALID_SOCKET) { int option_value = 1; From 97eea940f9d53a652bfcd3c6484a557377f07569 Mon Sep 17 00:00:00 2001 From: John Salem Date: Fri, 16 Jul 2021 16:11:44 -0700 Subject: [PATCH 3/4] fix compiler warning --- src/native/eventpipe/ds-ipc-pal-socket.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 1eac35d9601569..41f2b7bf50487c 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -323,7 +323,10 @@ ipc_socket_create_uds (DiagnosticsIpc *ipc) DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, 0); #ifndef SOCK_CLOEXEC - int fcntl_rc = fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value +#if DEBUG + int fcntl_rc = +#endif // DEBUG + fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value EP_ASSERT (fcntl_rc != -1); #endif // SOCK_CLOEXEC DS_EXIT_BLOCKING_PAL_SECTION; @@ -352,7 +355,10 @@ ipc_socket_create_tcp (DiagnosticsIpc *ipc) DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, IPPROTO_TCP); #ifndef SOCK_CLOEXEC - int fcntl_rc = fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value +#if DEBUG + int fcntl_rc = +#endif // DEBUG + fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value EP_ASSERT(fcntl_rc != -1); #endif // SOCK_CLOEXEC if (new_socket != DS_IPC_INVALID_SOCKET) { From b91e1a10a8a34634513b30c2b136f23d561436b1 Mon Sep 17 00:00:00 2001 From: John Salem Date: Fri, 16 Jul 2021 16:56:40 -0700 Subject: [PATCH 4/4] simplify asserts to appease all the different builds --- src/native/eventpipe/ds-ipc-pal-socket.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 41f2b7bf50487c..766aa320735c64 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -323,15 +323,15 @@ ipc_socket_create_uds (DiagnosticsIpc *ipc) DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, 0); #ifndef SOCK_CLOEXEC -#if DEBUG - int fcntl_rc = -#endif // DEBUG - fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value - EP_ASSERT (fcntl_rc != -1); + if (new_socket != DS_IPC_INVALID_SOCKET) { + if (fcntl (new_socket, F_SETFD, FD_CLOEXEC) == -1) { + EP_ASSERT (!"Failed to set CLOEXEC"); + } + } #endif // SOCK_CLOEXEC DS_EXIT_BLOCKING_PAL_SECTION; return new_socket; -#endif +#endif // DS_IPC_PAL_AF_UNIX return DS_IPC_INVALID_SOCKET; } @@ -354,14 +354,12 @@ ipc_socket_create_tcp (DiagnosticsIpc *ipc) #endif // SOCK_CLOEXEC DS_ENTER_BLOCKING_PAL_SECTION; new_socket = socket (ipc->server_address_family, socket_type, IPPROTO_TCP); + if (new_socket != DS_IPC_INVALID_SOCKET) { #ifndef SOCK_CLOEXEC -#if DEBUG - int fcntl_rc = -#endif // DEBUG - fcntl(new_socket, F_SETFD, FD_CLOEXEC); // best effort; don't validate return value - EP_ASSERT(fcntl_rc != -1); + if (fcntl (new_socket, F_SETFD, FD_CLOEXEC) == -1) { + EP_ASSERT (!"Failed to set CLOEXEC"); + } #endif // SOCK_CLOEXEC - if (new_socket != DS_IPC_INVALID_SOCKET) { int option_value = 1; setsockopt (new_socket, IPPROTO_TCP, TCP_NODELAY, (const char*)&option_value, sizeof (option_value)); if (ipc->mode == DS_IPC_CONNECTION_MODE_LISTEN) {