Fixes for copilot reviews

Author: dgarske

src/fwtpm/fwtpm_io.c

@@ -53,12 +53,22 @@
 
 #include <wolftpm/tpm2_socket.h>
 
+#include <signal.h> /* sig_atomic_t */
 #ifndef _WIN32
 #include <sys/select.h>
 #include <arpa/inet.h>
-#include <signal.h>
 #endif
 
+/* Async-signal-safe stop flag. Set by FWTPM_IO_RequestStop() (callable from
+ * a signal handler) and polled by the server loop. Using sig_atomic_t guarantees
+ * POSIX-conformant atomic read/write from inside a signal handler. */
+static volatile sig_atomic_t g_stopRequested = 0;
+
+void FWTPM_IO_RequestStop(void)
+{
+    g_stopRequested = 1;
+}
+
 /* Use TPM2_Packet_SwapU32 from tpm2_packet.c (compiled directly) */
 #define FwTpmSwapU32 TPM2_Packet_SwapU32
 
@@ -553,6 +563,12 @@ int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx)
      * the command connection open while sending POWER_ON on the platform port.
      * A sequential accept/handle loop would deadlock here. */
     while (ctx->running) {
+        /* Propagate async stop request into ctx state from normal
+         * control flow (signal handler only sets the volatile flag). */
+        if (g_stopRequested) {
+            ctx->running = 0;
+            break;
+        }
         FD_ZERO(&readFds);
         FD_SET(ctx->io.listenFd, &readFds);
         FD_SET(ctx->io.platListenFd, &readFds);

src/fwtpm/fwtpm_main.c

@@ -38,19 +38,13 @@
 #include <string.h>
 #include <signal.h>
 
-/* Global context pointer for signal handler.
- * Only the volatile flag is set in the handler; actual cleanup
- * happens in the main loop after the flag is observed. */
-static volatile sig_atomic_t g_terminate = 0;
-static FWTPM_CTX* g_ctx = NULL;
-
+/* Signal handler does only async-signal-safe work: it calls
+ * FWTPM_IO_RequestStop(), which sets a volatile sig_atomic_t. The server
+ * loop polls that flag and clears ctx->running from normal control flow. */
 static void sigterm_handler(int sig)
 {
     (void)sig;
-    g_terminate = 1;
-    if (g_ctx != NULL) {
-        g_ctx->running = 0;
-    }
+    FWTPM_IO_RequestStop();
 }
 
 static void usage(const char* progname)
@@ -159,7 +153,6 @@ int main(int argc, char* argv[])
     printf("  Model:         %s\n", FWTPM_MODEL);
 
     /* Install signal handler for graceful shutdown with NV save */
-    g_ctx = &ctx;
     sa.sa_handler = sigterm_handler;
     sigemptyset(&sa.sa_mask);
     sa.sa_flags = 0;

src/tpm2_swtpm.c

@@ -58,13 +58,19 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #endif
-#ifndef NO_GETENV
-#include <stdlib.h>
+#if !defined(NO_GETENV) || defined(WOLFTPM_SWTPM_UART)
+#include <stdlib.h> /* getenv / atoi */
 #endif
 #ifdef WOLFTPM_SWTPM_UART
 #include <fcntl.h>
 #include <termios.h>
 #include <sys/stat.h>
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 0
+#endif
+#ifndef O_NOFOLLOW
+#define O_NOFOLLOW 0
+#endif
 #endif
 
 #include <wolftpm/tpm2_socket.h>
@@ -185,6 +191,13 @@ static TPM_RC SwTpmConnect(TPM2_CTX* ctx, const char* host, const char* port)
     #endif
         return TPM_RC_FAILURE;
     }
+    /* If O_CLOEXEC wasn't available at compile time, set FD_CLOEXEC now. */
+    if (O_CLOEXEC == 0) {
+        int flags = fcntl(fd, F_GETFD);
+        if (flags != -1) {
+            (void)fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+        }
+    }
     /* Verify the opened path is a character device */
     if (fstat(fd, &devStat) != 0 || !S_ISCHR(devStat.st_mode)) {
         close(fd);

tests/fwtpm_check.sh

@@ -44,7 +44,8 @@ wait_for_port() {
     return 1
 }
 
-# Check if a port is in use (returns 0 if port is in use)
+# Check if a port is in use.
+# Returns: 0 = in use, 1 = free, 2 = unknown (no probe tool available)
 check_port_in_use() {
     local port="$1"
     if command -v nc >/dev/null 2>&1; then
@@ -53,22 +54,30 @@ check_port_in_use() {
     elif command -v ss >/dev/null 2>&1; then
         ss -tln 2>/dev/null | grep -q ":${port} "
         return $?
-    elif netstat -tln 2>/dev/null | grep -q ":${port} "; then
-        return 0
+    elif command -v netstat >/dev/null 2>&1; then
+        netstat -tln 2>/dev/null | grep -q ":${port} "
+        return $?
     fi
-    return 1  # no tool available, skip this port
+    return 2  # no probe tool — cannot determine
 }
 
 # Pick an available random port (returns port on stdout)
 pick_available_port() {
-    local port attempts=0
+    local port attempts=0 rv
     while [ $attempts -lt 20 ]; do
         if command -v shuf > /dev/null 2>&1; then
             port=$(shuf -i 10000-65000 -n 1)
         else
             port=$(( (RANDOM % 55000) + 10000 ))
         fi
-        if ! check_port_in_use "$port"; then
+        check_port_in_use "$port"; rv=$?
+        if [ $rv -eq 1 ]; then
+            echo "$port"
+            return 0
+        fi
+        if [ $rv -eq 2 ]; then
+            # No probe tool — accept the port; bind-time conflicts will surface
+            # as a server startup error rather than silent flakiness.
             echo "$port"
             return 0
         fi
@@ -270,13 +279,21 @@ if [ $IS_FWTPM_MODE -eq 1 ]; then
         rm -f "$BUILD_DIR"/certs/tpm-*-cert.pem "$BUILD_DIR"/certs/tpm-*-cert.csr
         rm -f "$BUILD_DIR"/certs/server-*-cert.pem "$BUILD_DIR"/certs/client-*-cert.pem
 
-        # Clean up any stale PID files from prior crashed runs
+        # Clean up any stale PID files from prior crashed runs.
+        # Validate the process is actually fwtpm_server before killing —
+        # PIDs can be reused, and signalling an unrelated process would
+        # cause collateral damage.
         for stale_pid_file in /tmp/fwtpm_check_*.pid; do
             [ -f "$stale_pid_file" ] || continue
             stale_pid="$(cat "$stale_pid_file" 2>/dev/null)"
             if [ -n "$stale_pid" ] && kill -0 "$stale_pid" 2>/dev/null; then
-                kill "$stale_pid" 2>/dev/null
-                sleep 0.3
+                stale_comm="$(ps -p "$stale_pid" -o comm= 2>/dev/null)"
+                case "$stale_comm" in
+                    fwtpm_server|*fwtpm_server*)
+                        kill "$stale_pid" 2>/dev/null
+                        sleep 0.3
+                        ;;
+                esac
             fi
             rm -f "$stale_pid_file"
         done

tests/fwtpm_unit_tests.c

@@ -895,7 +895,7 @@ static int BuildCreatePrimaryCmd(byte* buf, TPM_ALG_ID algType)
     return pos;
 }
 
-#ifndef NO_RSA
+#if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN)
 static void test_fwtpm_create_primary_rsa(void)
 {
     FWTPM_CTX ctx;
@@ -926,7 +926,7 @@ static void test_fwtpm_create_primary_rsa(void)
     FWTPM_Cleanup(&ctx);
     printf("Test fwTPM:\tCreatePrimary(RSA-2048):\t\tPassed\n");
 }
-#endif /* !NO_RSA */
+#endif /* !NO_RSA && WOLFSSL_KEY_GEN */
 
 #ifdef HAVE_ECC
 static void test_fwtpm_create_primary_ecc(void)
@@ -1996,7 +1996,7 @@ int fwtpm_unit_tests(int argc, char *argv[])
     test_fwtpm_clock_set();
 
     /* Key operations */
-#ifndef NO_RSA
+#if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN)
     test_fwtpm_create_primary_rsa();
 #endif
 #ifdef HAVE_ECC

wolftpm/fwtpm/fwtpm_io.h

@@ -59,9 +59,14 @@ WOLFTPM_API int FWTPM_IO_Init(FWTPM_CTX* ctx);
 /* Cleanup sockets */
 WOLFTPM_API void FWTPM_IO_Cleanup(FWTPM_CTX* ctx);
 
-/* Main server loop - blocks until ctx->running is cleared */
+/* Main server loop - blocks until ctx->running is cleared or stop requested */
 WOLFTPM_API int FWTPM_IO_ServerLoop(FWTPM_CTX* ctx);
 
+/* Async-signal-safe: request the server loop to exit. Safe to call from
+ * a signal handler — only sets a volatile sig_atomic_t flag that the
+ * main loop polls. */
+WOLFTPM_API void FWTPM_IO_RequestStop(void);
+
 #ifdef __cplusplus
     }  /* extern "C" */
 #endif