Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/ic tpm(4): Fix suspend and rework I/O transaction lock.



details:   https://anonhg.NetBSD.org/src/rev/e7942ae844fb
branches:  trunk
changeset: 359679:e7942ae844fb
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Jan 16 20:24:34 2022 +0000

description:
tpm(4): Fix suspend and rework I/O transaction lock.

Use sc->sc_lock over individual I/O transactions, not open/close of
the whole device.  This way there is a bounded time before the tpm is
unbusied even if userland is getting at it, so userland can't hold up
suspend indefinitely.  Of course, the tpm might be suspended and
resumed in the middle of the user's session this way -- tough.

This limits the response buffer to 1024 bytes -- which is already a
bit hefty to have on the stack (but it's probably not very deep on
the stack from userland so maybe not a big deal).  If it turns out we
need more, we can use kmem to allocate a buffer on the heap, with the
caveat that it might fail.  This is necessary so that suspend doesn't
block indefinitely on uiomove in tpmread.

diffstat:

 sys/dev/ic/tpm.c |  263 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 214 insertions(+), 49 deletions(-)

diffs (truncated from 365 to 300 lines):

diff -r 483c1d6b2658 -r e7942ae844fb sys/dev/ic/tpm.c
--- a/sys/dev/ic/tpm.c  Sun Jan 16 19:04:00 2022 +0000
+++ b/sys/dev/ic/tpm.c  Sun Jan 16 20:24:34 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tpm.c,v 1.23 2021/12/20 23:05:55 riastradh Exp $       */
+/*     $NetBSD: tpm.c,v 1.24 2022/01/16 20:24:34 riastradh Exp $       */
 
 /*
  * Copyright (c) 2019 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tpm.c,v 1.23 2021/12/20 23:05:55 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tpm.c,v 1.24 2022/01/16 20:24:34 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -141,14 +141,91 @@
                0x00, 0x00, 0x00, 0x98  /* TPM_ORD_SaveState */
        };
        struct tpm_header response;
+       size_t nread;
+       bool endwrite = false, endread = false;
+       int error;
 
-       if ((*sc->sc_intf->write)(sc, &command, sizeof(command)) != 0)
+       /*
+        * Write the command.
+        */
+       error = (*sc->sc_intf->start)(sc, UIO_WRITE);
+       if (error) {
+               device_printf(sc->sc_dev, "start write failed: %d", error);
+               goto out;
+       }
+
+       endwrite = true;
+
+       error = (*sc->sc_intf->write)(sc, &command, sizeof(command));
+       if (error) {
+               device_printf(sc->sc_dev, "write TPM_ORD_SaveState failed: %d",
+                   error);
+               goto out;
+       }
+
+       endwrite = false;
+
+       error = (*sc->sc_intf->end)(sc, UIO_WRITE, 0);
+       if (error) {
+               device_printf(sc->sc_dev, "end write failed: %d", error);
+               goto out;
+       }
+
+       /*
+        * Read the response -- just the header; we don't expect a
+        * payload.
+        */
+       error = (*sc->sc_intf->start)(sc, UIO_READ);
+       if (error) {
+               device_printf(sc->sc_dev, "start read failed: %d", error);
+               goto out;
+       }
+
+       endread = true;
+
+       error = (*sc->sc_intf->read)(sc, &response, sizeof(response), &nread,
+           0);
+       if (error) {
+               device_printf(sc->sc_dev, "read failed: %d", error);
+               goto out;
+       }
+       if (nread != sizeof(response)) {
+               device_printf(sc->sc_dev, "short header read: %zu", nread);
+               goto out;
+       }
+
+       endread = false;
+
+       error = (*sc->sc_intf->end)(sc, UIO_READ, 0);
+       if (error) {
+               device_printf(sc->sc_dev, "end read failed: %d", error);
+               goto out;
+       }
+
+       /*
+        * Verify the response looks reasonable.
+        */
+       if (TPM_BE16(response.tag) != TPM_TAG_RSP_COMMAND ||
+           TPM_BE32(response.length) != sizeof(response) ||
+           TPM_BE32(response.code) != 0) {
+               device_printf(sc->sc_dev,
+                   "TPM_ORD_SaveState failed: tag=0x%x length=0x%x code=0x%x",
+                   TPM_BE16(response.tag),
+                   TPM_BE32(response.length),
+                   TPM_BE32(response.code));
+               error = EIO;
+               goto out;
+       }
+
+       /* Success!  */
+       error = 0;
+
+out:   if (endwrite)
+               error = (*sc->sc_intf->end)(sc, UIO_WRITE, error);
+       if (endread)
+               error = (*sc->sc_intf->end)(sc, UIO_READ, error);
+       if (error)
                return false;
-       if ((*sc->sc_intf->read)(sc, &response, sizeof(response), NULL, 0) != 0)
-               return false;
-       if (TPM_BE32(response.code) != 0)
-               return false;
-
        return true;
 }
 
@@ -162,14 +239,91 @@
                0x00, 0x01              /* TPM_SU_STATE */
        };
        struct tpm_header response;
+       size_t nread;
+       bool endwrite = false, endread = false;
+       int error;
 
-       if ((*sc->sc_intf->write)(sc, &command, sizeof(command)) != 0)
+       /*
+        * Write the command.
+        */
+       error = (*sc->sc_intf->start)(sc, UIO_WRITE);
+       if (error) {
+               device_printf(sc->sc_dev, "start write failed: %d", error);
+               goto out;
+       }
+
+       endwrite = true;
+
+       error = (*sc->sc_intf->write)(sc, &command, sizeof(command));
+       if (error) {
+               device_printf(sc->sc_dev, "write TPM_ORD_SaveState failed: %d",
+                   error);
+               goto out;
+       }
+
+       endwrite = false;
+
+       error = (*sc->sc_intf->end)(sc, UIO_WRITE, 0);
+       if (error) {
+               device_printf(sc->sc_dev, "end write failed: %d", error);
+               goto out;
+       }
+
+       /*
+        * Read the response -- just the header; we don't expect a
+        * payload.
+        */
+       error = (*sc->sc_intf->start)(sc, UIO_READ);
+       if (error) {
+               device_printf(sc->sc_dev, "start read failed: %d", error);
+               goto out;
+       }
+
+       endread = true;
+
+       error = (*sc->sc_intf->read)(sc, &response, sizeof(response), &nread,
+           0);
+       if (error) {
+               device_printf(sc->sc_dev, "read failed: %d", error);
+               goto out;
+       }
+       if (nread != sizeof(response)) {
+               device_printf(sc->sc_dev, "short header read: %zu", nread);
+               goto out;
+       }
+
+       endread = false;
+
+       error = (*sc->sc_intf->end)(sc, UIO_READ, 0);
+       if (error) {
+               device_printf(sc->sc_dev, "end read failed: %d", error);
+               goto out;
+       }
+
+       /*
+        * Verify the response looks reasonable.
+        */
+       if (TPM_BE16(response.tag) != TPM2_ST_NO_SESSIONS ||
+           TPM_BE32(response.length) != sizeof(response) ||
+           TPM_BE32(response.code) != TPM2_RC_SUCCESS) {
+               device_printf(sc->sc_dev,
+                   "TPM_CC_Shutdown failed: tag=0x%x length=0x%x code=0x%x",
+                   TPM_BE16(response.tag),
+                   TPM_BE32(response.length),
+                   TPM_BE32(response.code));
+               error = EIO;
+               goto out;
+       }
+
+       /* Success!  */
+       error = 0;
+
+out:   if (endwrite)
+               error = (*sc->sc_intf->end)(sc, UIO_WRITE, error);
+       if (endread)
+               error = (*sc->sc_intf->end)(sc, UIO_READ, error);
+       if (error)
                return false;
-       if ((*sc->sc_intf->read)(sc, &response, sizeof(response), NULL, 0) != 0)
-               return false;
-       if (TPM_BE32(response.code) != 0)
-               return false;
-
        return true;
 }
 
@@ -608,21 +762,13 @@
 {
        struct tpm_softc *sc = cookie;
        unsigned nbytes, entropybits;
-       bool busy;
        int rv;
 
        /* Acknowledge the request.  */
        nbytes = atomic_swap_uint(&sc->sc_rndpending, 0);
 
-       /* Lock userland out of the tpm, or fail if it's already open.  */
+       /* Lock the tpm while we do I/O transactions with it.  */
        mutex_enter(&sc->sc_lock);
-       busy = sc->sc_busy;
-       sc->sc_busy = true;
-       mutex_exit(&sc->sc_lock);
-       if (busy) {             /* tough */
-               aprint_debug_dev(sc->sc_dev, "%s: device in use\n", __func__);
-               return;
-       }
 
        /*
         * Issue as many commands as needed to fulfill the request, but
@@ -654,10 +800,7 @@
                /* XXX worker thread can't workqueue_destroy its own queue */
        }
 
-       /* Relinquish the tpm back to userland.  */
-       mutex_enter(&sc->sc_lock);
-       KASSERT(sc->sc_busy);
-       sc->sc_busy = false;
+       /* Relinquish the tpm.  */
        mutex_exit(&sc->sc_lock);
 }
 
@@ -920,46 +1063,60 @@
        struct tpm_softc *sc = device_lookup_private(&tpm_cd, minor(dev));
        struct tpm_header hdr;
        uint8_t buf[TPM_BUFSIZ];
-       size_t cnt, len, n;
+       size_t cnt, len = 0/*XXXGCC*/;
+       bool end = false;
        int rv;
 
        if (sc == NULL)
                return ENXIO;
 
+       mutex_enter(&sc->sc_lock);
+
        if ((rv = (*sc->sc_intf->start)(sc, UIO_READ)))
-               return rv;
+               goto out;
+       end = true;
 
        /* Get the header. */
        if ((rv = (*sc->sc_intf->read)(sc, &hdr, sizeof(hdr), &cnt, 0))) {
                goto out;
        }
+       if (cnt != sizeof(hdr)) {
+               rv = EIO;
+               goto out;
+       }
        len = TPM_BE32(hdr.length);
-       if (len > uio->uio_resid || len < cnt) {
+       if (len > MIN(sizeof(buf), uio->uio_resid) || len < sizeof(hdr)) {
                rv = EIO;
                goto out;
        }
 
-       /* Copy out the header. */
-       if ((rv = uiomove(&hdr, cnt, uio))) {
+       /* Get the payload. */
+       len -= sizeof(hdr);
+       if ((rv = (*sc->sc_intf->read)(sc, buf, len, NULL, TPM_PARAM_SIZE))) {
                goto out;
        }
 
-       /* Process the rest. */
-       len -= cnt;
-       while (len > 0) {
-               n = MIN(sizeof(buf), len);
-               if ((rv = (*sc->sc_intf->read)(sc, buf, n, NULL, TPM_PARAM_SIZE))) {
-                       goto out;
-               }
-               if ((rv = uiomove(buf, n, uio))) {
-                       goto out;



Home | Main Index | Thread Index | Old Index