CI and peer review fixes

Author: dgarske

.github/workflows/fwtpm-test.yml

@@ -389,20 +389,36 @@ jobs:
               CFLAGS="${{ matrix.extra_cflags }} -g -O1"
           make
 
+      - name: Start fwtpm_server
+        run: |
+          rm -f fwtpm_nv.bin
+          ./src/fwtpm/fwtpm_server > /tmp/fwtpm_server.log 2>&1 &
+          echo $! > /tmp/fwtpm_server.pid
+          sleep 1
+          kill -0 $(cat /tmp/fwtpm_server.pid)
+
       - name: Run unit.test under valgrind
         run: |
           valgrind --error-exitcode=1 --leak-check=full \
                    --errors-for-leak-kinds=definite \
                    --show-leak-kinds=definite \
                    ./tests/unit.test
 
+      - name: Stop fwtpm_server
+        if: always()
+        run: |
+          if [ -f /tmp/fwtpm_server.pid ]; then
+            kill $(cat /tmp/fwtpm_server.pid) 2>/dev/null || true
+          fi
+
       - name: Upload failure logs
         if: failure()
         uses: actions/upload-artifact@v4
         with:
           name: fwtpm-valgrind-logs-${{ matrix.name }}
           path: |
             /tmp/fwtpm_check_*.log
+            /tmp/fwtpm_server.log
             test-suite.log
             tests/*.log
             config.log

src/fwtpm/fwtpm_tis.c

@@ -411,6 +411,7 @@ int FWTPM_TIS_ServerLoop(FWTPM_CTX* ctx)
 {
     FWTPM_TIS_HAL* hal;
     FWTPM_TIS_REGS* regs;
+    TPM_RC retRc = TPM_RC_SUCCESS;
     int rc;
 #ifndef _WIN32
     struct sigaction sa;
@@ -441,7 +442,10 @@ int FWTPM_TIS_ServerLoop(FWTPM_CTX* ctx)
             continue; /* EINTR or transient, retry */
         }
         if (rc < 0) {
-            break; /* fatal error */
+            /* Fatal HAL error — propagate so the caller can distinguish
+             * this from a clean shutdown via ctx->running=0. */
+            retRc = TPM_RC_FAILURE;
+            break;
         }
 
         /* Process the register access */
@@ -453,7 +457,7 @@ int FWTPM_TIS_ServerLoop(FWTPM_CTX* ctx)
         }
     }
 
-    return TPM_RC_SUCCESS;
+    return retRc;
 }
 
 #endif /* WOLFTPM_FWTPM && WOLFTPM_FWTPM_TIS */

src/fwtpm/fwtpm_tis_shm.c

@@ -68,6 +68,11 @@ static int TisShmInit(void* ctx, FWTPM_TIS_REGS** regs)
 {
     FWTPM_TIS_SHM_CTX* shm = (FWTPM_TIS_SHM_CTX*)ctx;
     int fd;
+    /* Threat model: fwtpm_server is a dev/test tool and is NOT intended to
+     * run setuid or as a privileged daemon. O_NOFOLLOW + mode 0600 is
+     * sufficient for non-privileged execution. We intentionally avoid
+     * O_EXCL so the server can recover from a prior unclean shutdown
+     * without manual cleanup of the shm file. */
     int openFlags = O_CREAT | O_RDWR | O_TRUNC;
 
 #ifdef O_NOFOLLOW

src/tpm2_packet.c

@@ -60,18 +60,26 @@ void TPM2_Packet_U64ToByteArray(UINT64 val, BYTE* b)
     }
 }
 
-/* Big-endian byte-array load helpers */
+/* Big-endian byte-array load helpers. Mirror the NULL-guard convention of the
+ * U*ToByteArray store helpers above so callers get 0 for a NULL input rather
+ * than a crash. */
 UINT16 TPM2_Packet_ByteArrayToU16(const BYTE* b)
 {
+    if (b == NULL)
+        return 0;
     return (UINT16)(((UINT16)b[0] << 8) | b[1]);
 }
 UINT32 TPM2_Packet_ByteArrayToU32(const BYTE* b)
 {
+    if (b == NULL)
+        return 0;
     return ((UINT32)b[0] << 24) | ((UINT32)b[1] << 16) |
            ((UINT32)b[2] << 8)  | b[3];
 }
 UINT64 TPM2_Packet_ByteArrayToU64(const BYTE* b)
 {
+    if (b == NULL)
+        return 0;
     return ((UINT64)b[0] << 56) | ((UINT64)b[1] << 48) |
            ((UINT64)b[2] << 40) | ((UINT64)b[3] << 32) |
            ((UINT64)b[4] << 24) | ((UINT64)b[5] << 16) |
@@ -258,7 +266,9 @@ void TPM2_Packet_ParseBytes(TPM2_Packet* packet, byte* buf, int size)
 void TPM2_Packet_ParseU16Buf(TPM2_Packet* packet, UINT16* size, byte* buf,
     UINT16 maxBufSz)
 {
-    UINT16 wireSize;
+    /* Init to 0 so a NULL packet (TPM2_Packet_ParseU16 is a no-op in that
+     * case) leaves wireSize well-defined for the arithmetic below. */
+    UINT16 wireSize = 0;
     UINT16 copySz;
 
     TPM2_Packet_ParseU16(packet, &wireSize);
@@ -809,6 +819,9 @@ TPM_RC TPM2_Packet_ParseSensitiveCreate(TPM2_Packet* packet, int maxSize,
     UINT16 dataSz = 0;
     int sensStartPos;
 
+    if (packet == NULL || userAuth == NULL) {
+        return BAD_FUNC_ARG;
+    }
     if (packet->pos + 2 > maxSize) {
         rc = TPM_RC_COMMAND_SIZE;
     }

tests/unit_tests.c

@@ -910,7 +910,7 @@ static void test_TPM2_HmacCompute(void)
         digest, &digestSz);
     AssertIntEQ(0, rc);
     AssertIntEQ(32, (int)digestSz);
-    AssertIntEQ(XMEMCMP(digest, hmacExp, sizeof(hmacExp)), 0);
+    AssertIntEQ(0, XMEMCMP(digest, hmacExp, sizeof(hmacExp)));
 
     /* Test HmacVerify with correct expected value */
     rc = TPM2_HmacVerify(TPM_ALG_SHA256,