git: 5319035afacc - main - tpm: Move user copies outside the TPM 1.2 lock

From: Kevin Bowling <kbowling_at_FreeBSD.org>
Date: Tue, 01 Sep 2026 18:25:53 UTC
The branch main has been updated by kbowling:

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

commit 5319035afacceaa267792ec193eb03fa51c972f7
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-27 14:01:51 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-01 18:24:46 +0000

    tpm: Move user copies outside the TPM 1.2 lock
    
    The character-device paths held the transaction and lifecycle lock while
    uiomove() accessed user memory.  A user page fault could therefore delay
    suspend or detach, and a copyout failure occurred while the TPM response
    was still active.
    
    Copy commands into the bounded stack buffer before taking the lock.  For
    reads, validate the response header, buffer the complete response while
    the lock is held, finish the TPM transaction, and copy it to userspace
    after unlocking.  Use a non-blocking allocation so memory pressure
    cannot turn response buffering into another lifecycle wait.
    
    NetBSD uses the same separation but limits responses to its fixed 1 KiB
    buffer.  Allocate the TPM-advertised response length to preserve the
    existing FreeBSD support for larger streamed responses.
    
    On a ThinkPad T440p with an STMicro TPM 1.2, a PCR read into a 4 KiB
    userspace buffer returned the expected 30-byte response.  A deliberately
    short five-byte read failed cleanly, relinquished locality zero, and the
    next PCR read succeeded.
    
    Reviewed by:    kevans
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D59238
---
 sys/dev/tpm/tpm.c | 71 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/sys/dev/tpm/tpm.c b/sys/dev/tpm/tpm.c
index f37ce6955f17..ee886e6ae7f8 100644
--- a/sys/dev/tpm/tpm.c
+++ b/sys/dev/tpm/tpm.c
@@ -40,6 +40,7 @@
 #include <isa/isavar.h>
 #include <dev/tpm/tpmvar.h>
 
+MALLOC_DEFINE(M_TPM, "tpm12_buffer", "buffer for TPM 1.2 responses");
 
 #define	TPM_BUFSIZ	1024
 
@@ -1473,13 +1474,14 @@ int
 tpmread(struct cdev *dev, struct uio *uio, int flags)
 {
 	struct tpm_softc *sc;
-	u_int8_t buf[TPM_BUFSIZ], *p;
-	size_t cnt;
-	int end_error, len, n, rv;
+	u_int8_t header[TPM_HDRSIZE], *buf;
+	size_t cnt, len;
+	int end_error, rv;
 
 	sc = TPMSOFTC(dev);
 	if (sc == NULL)
 		return (ENXIO);
+	buf = NULL;
 
 	sx_xlock(&sc->sc_lock);
 	if (sc->sc_dying) {
@@ -1497,39 +1499,42 @@ tpmread(struct cdev *dev, struct uio *uio, int flags)
 #ifdef TPM_DEBUG
 	printf("tpmread: getting header\n");
 #endif
-	rv = sc->sc_read(sc, buf, TPM_HDRSIZE, &cnt, 0);
+	rv = sc->sc_read(sc, header, sizeof(header), &cnt,
+	    TPM_PARAM_SIZE);
 	if (rv != 0)
 		goto end;
+	if (cnt != sizeof(header)) {
+		rv = EIO;
+		goto end;
+	}
 
-	len = (buf[2] << 24) | (buf[3] << 16) | (buf[4] << 8) | buf[5];
+	len = be32dec(header + 2);
 #ifdef TPM_DEBUG
-	printf("tpmread: len %d, io count %d\n", len, uio->uio_resid);
+	printf("tpmread: len %zu, io count %zd\n", len, uio->uio_resid);
 #endif
-	if (len > uio->uio_resid) {
+	if (len < sizeof(header) || len > uio->uio_resid || len > INT_MAX) {
 		rv = EIO;
 #ifdef TPM_DEBUG
-		printf("tpmread: bad residual io count 0x%x\n", uio->uio_resid);
+		printf("tpmread: invalid response length %zu\n", len);
 #endif
 		goto end;
 	}
 
-	/* Copy out header. */
-	rv = uiomove((caddr_t)buf, cnt, uio);
-	if (rv != 0)
+	/*
+	 * Finish the device transaction before touching user memory.  Use a
+	 * non-blocking allocation so lifecycle operations are not held up by
+	 * memory pressure while waiting for the transaction lock.
+	 */
+	buf = malloc(len, M_TPM, M_NOWAIT);
+	if (buf == NULL) {
+		rv = ENOMEM;
 		goto end;
+	}
+	memcpy(buf, header, sizeof(header));
 
-	/* Get remaining part of the answer (if anything is left). */
-	for (len -= cnt, p = buf, n = sizeof(buf); len > 0; p = buf, len -= n,
-	    n = sizeof(buf)) {
-		n = MIN(n, len);
-#ifdef TPM_DEBUG
-		printf("tpmread: n %d len %d\n", n, len);
-#endif
-		rv = sc->sc_read(sc, p, n, NULL, TPM_PARAM_SIZE);
-		if (rv != 0)
-			goto end;
-		p += n;
-		rv = uiomove((caddr_t)buf, p - buf, uio);
+	if (len > sizeof(header)) {
+		rv = sc->sc_read(sc, buf + sizeof(header),
+		    (int)(len - sizeof(header)), NULL, TPM_PARAM_SIZE);
 		if (rv != 0)
 			goto end;
 	}
@@ -1540,6 +1545,9 @@ end:
 		rv = end_error;
 out:
 	sx_xunlock(&sc->sc_lock);
+	if (rv == 0)
+		rv = uiomove(buf, (int)len, uio);
+	free(buf, M_TPM);
 	return (rv);
 }
 
@@ -1548,11 +1556,19 @@ tpmwrite(struct cdev *dev, struct uio *uio, int flags)
 {
 	struct tpm_softc *sc;
 	u_int8_t buf[TPM_BUFSIZ];
+	ssize_t resid;
 	int end_error, n, rv;
 
 	sc = TPMSOFTC(dev);
 	if (sc == NULL)
 		return (ENXIO);
+
+	resid = uio->uio_resid;
+	n = MIN(sizeof(buf), resid);
+	rv = uiomove(buf, n, uio);
+	if (rv != 0)
+		return (rv);
+
 	sx_xlock(&sc->sc_lock);
 	if (sc->sc_dying) {
 		rv = ENXIO;
@@ -1564,14 +1580,9 @@ tpmwrite(struct cdev *dev, struct uio *uio, int flags)
 	}
 
 #ifdef TPM_DEBUG
-	printf("tpmwrite: io count %d\n", uio->uio_resid);
+	printf("tpmwrite: io count %d\n", n);
 #endif
 
-	n = MIN(sizeof(buf), uio->uio_resid);
-	rv = uiomove((caddr_t)buf, n, uio);
-	if (rv != 0)
-		goto out;
-
 	rv = sc->sc_start(sc, UIO_WRITE);
 	if (rv != 0)
 		goto out;
@@ -1583,6 +1594,8 @@ tpmwrite(struct cdev *dev, struct uio *uio, int flags)
 
 out:
 	sx_xunlock(&sc->sc_lock);
+	if (rv != 0)
+		uio->uio_resid = resid;
 	return (rv);
 }