git: 5319035afacc - main - tpm: Move user copies outside the TPM 1.2 lock
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);
}