git: a3daa1e8f598 - main - tpm20: Harden the common device lifecycle

From: Kevin Bowling <kbowling_at_FreeBSD.org>
Date: Thu, 03 Sep 2026 03:58:23 UTC
The branch main has been updated by kbowling:

URL: https://cgit.FreeBSD.org/src/commit/?id=a3daa1e8f5988045e8042fb3fe376384a43b85ee

commit a3daa1e8f5988045e8042fb3fe376384a43b85ee
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-27 12:42:28 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-03 03:56:23 +0000

    tpm20: Harden the common device lifecycle
    
    Mark the device as dying before teardown and destroy the character
    device before freeing its private state or lock.  This prevents cdev
    methods from entering with a freed internal buffer or a destroyed sx.
    
    Check the teardown state in command paths, honor failures from the cdev
    private data interface, and publish teardown before waiting for the
    lifecycle lock.  Keep that lock across TPM retry delays so commands
    cannot interpose and private state remains pinned, but abort before the
    next retry once teardown begins.
    
    Block new cdev operations after a successful Shutdown(STATE).  Keep the
    suspend gate and the TPM command under the same lock so a userspace
    command cannot invalidate the saved state before S3 entry.  Clear the
    gate only after Startup(STATE) succeeds.
    
    Keep entropy harvesting scheduled after a transient command or suspend
    failure, but stop it while suspended or once teardown begins.  Queue the
    next timeout while holding the lifecycle lock so release cannot miss a
    concurrent requeue.
    
    Validated on two TPM 2.0 FIFO systems.  Each completed five device
    suspend/resume cycles, rejected both new and already-open cdev
    operations with EBUSY while suspended, completed 200 concurrent PCR
    reads, and detached cleanly while four PCR readers were active.  A
    ThinkPad P51 also completed a full S3 cycle with PCR 0 unchanged and
    50 successful reads after resume.
    
    Reviewed by:    kevans
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D59240
---
 sys/dev/tpm/tpm20.c | 168 ++++++++++++++++++++++++++++++++++++++--------------
 sys/dev/tpm/tpm20.h |   2 +
 2 files changed, 125 insertions(+), 45 deletions(-)

diff --git a/sys/dev/tpm/tpm20.c b/sys/dev/tpm/tpm20.c
index 20521b2ed361..bebf0af400fa 100644
--- a/sys/dev/tpm/tpm20.c
+++ b/sys/dev/tpm/tpm20.c
@@ -28,6 +28,8 @@
 #include <sys/random.h>
 #include <dev/random/randomdev.h>
 
+#include <machine/atomic.h>
+
 #include "tpm20.h"
 
 #define TPM_HARVEST_SIZE     16
@@ -84,12 +86,22 @@ tpm20_read(struct cdev *dev, struct uio *uio, int flags)
 	struct tpm_priv *priv;
 	size_t bytes_to_transfer;
 	size_t offset;
-	int result = 0;
+	int result;
 
 	sc = (struct tpm_sc *)dev->si_drv1;
-	devfs_get_cdevpriv((void **)&priv);
+	result = devfs_get_cdevpriv((void **)&priv);
+	if (result != 0)
+		return (result);
 
 	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying)) {
+		result = ENXIO;
+		goto out;
+	}
+	if (sc->suspended) {
+		result = EBUSY;
+		goto out;
+	}
 	offset = priv->offset;
 	bytes_to_transfer = MIN(priv->len, uio->uio_resid);
 	if (bytes_to_transfer > 0) {
@@ -100,8 +112,8 @@ tpm20_read(struct cdev *dev, struct uio *uio, int flags)
 		result = 0;
 	}
 
+out:
 	sx_xunlock(&sc->dev_lock);
-
 	return (result);
 }
 
@@ -111,10 +123,12 @@ tpm20_write(struct cdev *dev, struct uio *uio, int flags)
 	struct tpm_sc *sc;
 	struct tpm_priv *priv;
 	size_t byte_count;
-	int result = 0;
+	int result;
 
 	sc = (struct tpm_sc *)dev->si_drv1;
-	devfs_get_cdevpriv((void **)&priv);
+	result = devfs_get_cdevpriv((void **)&priv);
+	if (result != 0)
+		return (result);
 
 	byte_count = uio->uio_resid;
 	if (byte_count < TPM_HEADER_SIZE) {
@@ -130,15 +144,22 @@ tpm20_write(struct cdev *dev, struct uio *uio, int flags)
 	}
 
 	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying)) {
+		result = ENXIO;
+		goto out;
+	}
+	if (sc->suspended) {
+		result = EBUSY;
+		goto out;
+	}
 
 	result = uiomove(priv->buf, byte_count, uio);
-	if (result != 0) {
-		sx_xunlock(&sc->dev_lock);
-		return (result);
-	}
+	if (result != 0)
+		goto out;
 
 	result = TPM_TRANSMIT(sc->dev, priv, byte_count);
 
+out:
 	sx_xunlock(&sc->dev_lock);
 	return (result);
 }
@@ -163,12 +184,28 @@ tpm20_priv_dtor(void *data)
 int
 tpm20_open(struct cdev *dev, int flag, int mode, struct thread *td)
 {
+	struct tpm_sc *sc;
 	struct tpm_priv *priv;
+	int error;
 
+	sc = (struct tpm_sc *)dev->si_drv1;
+	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying)) {
+		error = ENXIO;
+		goto out;
+	}
+	if (sc->suspended) {
+		error = EBUSY;
+		goto out;
+	}
 	priv = tpm20_priv_alloc();
-	devfs_set_cdevpriv(priv, tpm20_priv_dtor);
+	error = devfs_set_cdevpriv(priv, tpm20_priv_dtor);
+	if (error != 0)
+		tpm20_priv_dtor(priv);
 
-	return (0);
+out:
+	sx_xunlock(&sc->dev_lock);
+	return (error);
 }
 
 int
@@ -199,6 +236,8 @@ tpm20_init(struct tpm_sc *sc)
 	struct make_dev_args args;
 	int result;
 
+	atomic_store_bool(&sc->dying, false);
+	sc->suspended = false;
 	sc->internal_priv = tpm20_priv_alloc();
 
 	make_dev_args_init(&args);
@@ -227,11 +266,20 @@ void
 tpm20_release(struct tpm_sc *sc)
 {
 
+	/* Publish teardown so a retrying command stops using the device. */
+	atomic_store_bool(&sc->dying, true);
+	sx_xlock(&sc->dev_lock);
+	sx_xunlock(&sc->dev_lock);
+
+	/* Stop and drain character-device methods before freeing their state. */
+	if (sc->sc_cdev != NULL) {
+		destroy_dev(sc->sc_cdev);
+		sc->sc_cdev = NULL;
+	}
 	if (!sc->common_initialized)
 		goto out;
 #if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
-	if (device_is_attached(sc->dev))
-		taskqueue_drain_timeout(taskqueue_thread, &sc->harvest_task);
+	taskqueue_drain_timeout(taskqueue_thread, &sc->harvest_task);
 	random_source_deregister(&random_tpm);
 #endif
 	sc->common_initialized = false;
@@ -241,25 +289,29 @@ out:
 		sc->internal_priv = NULL;
 	}
 	sx_destroy(&sc->dev_lock);
-	if (sc->sc_cdev != NULL) {
-		destroy_dev(sc->sc_cdev);
-		sc->sc_cdev = NULL;
-	}
 }
 
 int
 tpm20_resume(device_t dev)
 {
+	struct tpm_sc *sc;
 	int error;
 
+	sc = device_get_softc(dev);
+	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying)) {
+		error = ENXIO;
+		goto out;
+	}
 	error = tpm20_restart(dev, false);
+	if (error == 0)
+		sc->suspended = false;
+out:
+	sx_xunlock(&sc->dev_lock);
 	if (error != 0)
 		return (error);
 
 #if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
-	struct tpm_sc *sc;
-
-	sc = device_get_softc(dev);
 	taskqueue_enqueue_timeout(taskqueue_thread, &sc->harvest_task,
 	    hz * TPM_HARVEST_INTERVAL);
 #endif
@@ -269,15 +321,27 @@ tpm20_resume(device_t dev)
 int
 tpm20_suspend(device_t dev)
 {
-	int error;
-
-#if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
 	struct tpm_sc *sc;
+	int error;
 
 	sc = device_get_softc(dev);
+#if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
 	taskqueue_drain_timeout(taskqueue_thread, &sc->harvest_task);
 #endif
+	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying)) {
+		error = ENXIO;
+		goto out;
+	}
+	if (sc->suspended) {
+		error = 0;
+		goto out;
+	}
 	error = tpm20_save_state(dev, true);
+	if (error == 0)
+		sc->suspended = true;
+out:
+	sx_xunlock(&sc->dev_lock);
 #if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
 	if (error != 0)
 		taskqueue_enqueue_timeout(taskqueue_thread, &sc->harvest_task,
@@ -289,7 +353,17 @@ tpm20_suspend(device_t dev)
 int
 tpm20_shutdown(device_t dev)
 {
-	return (tpm20_save_state(dev, false));
+	struct tpm_sc *sc;
+	int error;
+
+	sc = device_get_softc(dev);
+	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying))
+		error = ENXIO;
+	else
+		error = tpm20_save_state(dev, false);
+	sx_xunlock(&sc->dev_lock);
+	return (error);
 }
 
 #if defined TPM_HARVEST || defined RANDOM_ENABLE_TPM
@@ -314,31 +388,32 @@ tpm20_harvest(void *arg, int unused)
 
 	sc = arg;
 	sx_xlock(&sc->dev_lock);
+	if (atomic_load_bool(&sc->dying) || sc->suspended) {
+		sx_xunlock(&sc->dev_lock);
+		return;
+	}
 
 	priv = sc->internal_priv;
 	memcpy(priv->buf, cmd, sizeof(cmd));
 
+	entropy_size = 0;
 	result = TPM_TRANSMIT(sc->dev, priv, sizeof(cmd));
-	if (result != 0) {
-		sx_xunlock(&sc->dev_lock);
-		return;
-	}
-
-	/* The number of random bytes we got is placed right after the header */
-	entropy_size = (uint16_t) priv->buf[TPM_HEADER_SIZE + 1];
-	if (entropy_size > 0) {
-		entropy_size = MIN(entropy_size, TPM_HARVEST_SIZE);
-		memcpy(entropy,
-			priv->buf + TPM_HEADER_SIZE + sizeof(uint16_t),
-			entropy_size);
+	if (result == 0) {
+		/* The byte count is placed immediately after the header. */
+		entropy_size = (uint16_t)priv->buf[TPM_HEADER_SIZE + 1];
+		if (entropy_size > 0) {
+			entropy_size = MIN(entropy_size, TPM_HARVEST_SIZE);
+			memcpy(entropy,
+			    priv->buf + TPM_HEADER_SIZE + sizeof(uint16_t),
+			    entropy_size);
+		}
 	}
+	taskqueue_enqueue_timeout(taskqueue_thread, &sc->harvest_task,
+	    hz * TPM_HARVEST_INTERVAL);
 
 	sx_xunlock(&sc->dev_lock);
 	if (entropy_size > 0)
 		random_harvest_queue(entropy, entropy_size, RANDOM_PURE_TPM);
-
-	taskqueue_enqueue_timeout(taskqueue_thread, &sc->harvest_task,
-	    hz * TPM_HARVEST_INTERVAL);
 }
 #endif	/* TPM_HARVEST */
 
@@ -359,15 +434,20 @@ tpm20_command(device_t dev, uint32_t command, uint16_t parameter,
 	if (sc == NULL)
 		return (ENXIO);
 
+	sx_assert(&sc->dev_lock, SA_XLOCKED);
+	if (atomic_load_bool(&sc->dying))
+		return (ENXIO);
+	priv = sc->internal_priv;
+
 	be16enc(cmd, TPM2_ST_NO_SESSIONS);
 	be32enc(cmd + 2, sizeof(cmd));
 	be32enc(cmd + 6, command);
 	be16enc(cmd + 10, parameter);
 
-	sx_xlock(&sc->dev_lock);
-	priv = sc->internal_priv;
 	delay_ms = TPM2_RETRY_INITIAL_MS;
 	for (;;) {
+		if (atomic_load_bool(&sc->dying))
+			return (ENXIO);
 		memcpy(priv->buf, cmd, sizeof(cmd));
 		error = TPM_TRANSMIT(sc->dev, priv, sizeof(cmd));
 		if (error != 0)
@@ -388,15 +468,13 @@ tpm20_command(device_t dev, uint32_t command, uint16_t parameter,
 		if (response_rc != TPM2_RC_RETRY &&
 		    response_rc != TPM2_RC_TESTING)
 			break;
+		if (atomic_load_bool(&sc->dying))
+			return (ENXIO);
 		if (delay_ms > TPM2_RETRY_MAX_MS)
 			break;
-		sx_xunlock(&sc->dev_lock);
 		pause("tpm2retry", MAX(hz * delay_ms / 1000, 1));
-		sx_xlock(&sc->dev_lock);
 		delay_ms *= 2;
 	}
-	sx_xunlock(&sc->dev_lock);
-
 	if (error != 0) {
 		device_printf(dev, "%s command failed: %d\n", name, error);
 		return (error);
diff --git a/sys/dev/tpm/tpm20.h b/sys/dev/tpm/tpm20.h
index 3bf59112bed8..2aa23318b5e5 100644
--- a/sys/dev/tpm/tpm20.h
+++ b/sys/dev/tpm/tpm20.h
@@ -130,6 +130,8 @@ struct tpm_sc {
 	uint32_t	intr_mask;	/* Saved TIS interrupt configuration */
 	bool 		interrupts;
 	bool		common_initialized;
+	bool		dying;
+	bool		suspended;
 
 	struct tpm_priv *internal_priv;