⬢ DragonFlyBSD Kernel Audit
DF-4407 / fix.diff
← back to finding ↓ download raw
fix.diff for DF-4407 — serialize /dev/tpm open/close/read/write (LK_EXCLUSIVE),
mirroring upstream FreeBSD r365144 which added dev_lock to tpm(4).  Authored
against the read-only sys/ tree; never applied there.  fix_status: not_testable
(no TPM on the audit guest; see env.txt).

--- a/sys/dev/crypto/tpm/tpmvar.h
+++ b/sys/dev/crypto/tpm/tpmvar.h
@@ -22,6 +22,8 @@
 #define _TPMVAR_H
 
 struct tpm_softc {
+	struct lock sc_lock;	/* SMP serialization of cdev access */
+
 	void *sc_ih;
 
 	int	(*sc_init)(struct tpm_softc *, int, const char *);
--- a/sys/dev/crypto/tpm/tpm.c
+++ b/sys/dev/crypto/tpm/tpm.c
@@ -200,6 +200,9 @@ tpm_attach(device_t dev)
 	struct tpm_softc *sc = device_get_softc(dev);
 	int irq;
 
+	lockinit(&sc->sc_lock, "tpm dev lock", 0, LK_CANRECURSE);
+	sc->sc_flags = 0;
+
 	sc->mem_rid = 0;
 	sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &sc->mem_rid,
 	    RF_ACTIVE);
@@ -277,6 +280,8 @@ tpm_detach(device_t dev)
 	if(sc->sc_cdev){
 		destroy_dev(sc->sc_cdev);
 	}
+
+	lockuninit(&sc->sc_lock);
 
 	return 0;
 }
@@ -1150,18 +1155,25 @@ tpmopen(struct dev_open_args *ap)
 	cdev_t dev = ap->a_head.a_dev;
 	struct tpm_softc *sc = TPMSOFTC(dev);
 
-	if (!sc)
+	if (!sc)
 		return ENXIO;
 
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE);
 	if (sc->sc_flags & TPM_OPEN)
-		return EBUSY;
+		rv = EBUSY;
+	else {
+		sc->sc_flags |= TPM_OPEN;
+		rv = 0;
+	}
+	lockmgr(&sc->sc_lock, LK_RELEASE);
 
-	sc->sc_flags |= TPM_OPEN;
+	return rv;
+}
 
-	return 0;
-}
+int
+tpmclose(struct dev_close_args *ap)
+{
+	cdev_t dev = ap->a_head.a_dev;
+	struct tpm_softc *sc = TPMSOFTC(dev);
 
-int
-tpmclose(struct dev_close_args *ap)
-{
-	cdev_t dev = ap->a_head.a_dev;
-	struct tpm_softc *sc = TPMSOFTC(dev);
+	if (!sc)
+		return ENXIO;
 
 	if (!sc)
 		return ENXIO;
@@ -1172,8 +1184,9 @@ tpmclose(struct dev_close_args *ap)
 	if (!(sc->sc_flags & TPM_OPEN))
 		return EINVAL;
 
-	sc->sc_flags &= ~TPM_OPEN;
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE);
+	sc->sc_flags &= ~TPM_OPEN;
+	lockmgr(&sc->sc_lock, LK_RELEASE);
 
 	return 0;
 }
@@ -1190,8 +1203,10 @@ tpmread(struct dev_read_args *ap)
 	if (!sc)
 		return ENXIO;
 
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE);
 	crit_enter();
-	if ((rv = (sc->sc_start)(sc, UIO_READ))) {
+	if ((rv = (sc->sc_start)(sc, UIO_READ))) {
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
@@ -1202,6 +1217,7 @@ tpmread(struct dev_read_args *ap)
 #endif
 	if ((rv = (sc->sc_read)(sc, buf, TPM_HDRSIZE, &cnt, 0))) {
 		(sc->sc_end)(sc, UIO_READ, rv);
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
@@ -1214,6 +1230,7 @@ tpmread(struct dev_read_args *ap)
 #endif
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
@@ -1224,6 +1241,7 @@ tpmread(struct dev_read_args *ap)
 	/* Copy out header. */
 	if ((rv = uiomove((caddr_t)buf, cnt, uio))) {
 		(sc->sc_end)(sc, UIO_READ, rv);
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
@@ -1237,6 +1255,7 @@ tpmread(struct dev_read_args *ap)
 		if ((rv = (sc->sc_read)(sc, p, n, TPM_PARAM_SIZE))) {
 			(sc->sc_end)(sc, UIO_READ, rv);
 			crit_exit();
+			lockmgr(&sc->sc_lock, LK_RELEASE);
 			return rv;
 		}
@@ -1243,12 +1262,15 @@ tpmread(struct dev_read_args *ap)
 		if ((rv = uiomove((caddr_t)buf, p - buf, uio))) {
 			(sc->sc_end)(sc, UIO_READ, rv);
 			crit_exit();
+			lockmgr(&sc->sc_lock, LK_RELEASE);
 			return rv;
 		}
 	}
 
 	rv = (sc->sc_end)(sc, UIO_READ, rv);
 	crit_exit();
+	lockmgr(&sc->sc_lock, LK_RELEASE);
+
 	return rv;
 }
@@ -1266,8 +1288,10 @@ tpmwrite(struct dev_write_args *ap)
 	if (!sc)
 		return ENXIO;
 
+	lockmgr(&sc->sc_lock, LK_EXCLUSIVE);
 	crit_enter();
 
@@ -1274,6 +1298,7 @@ tpmwrite(struct dev_write_args *ap)
 	n = MIN(sizeof(buf), uio->uio_resid);
 	if ((rv = uiomove((caddr_t)buf, n, uio))) {
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
@@ -1279,14 +1304,18 @@ tpmwrite(struct dev_write_args *ap)
 	if ((rv = (sc->sc_start)(sc, UIO_WRITE))) {
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
 
 	if ((rv = (sc->sc_write(sc, buf, n)))) {
 		crit_exit();
+		lockmgr(&sc->sc_lock, LK_RELEASE);
 		return rv;
 	}
 
 	rv = (sc->sc_end)(sc, UIO_WRITE, rv);
 	crit_exit();
+	lockmgr(&sc->sc_lock, LK_RELEASE);
+
 	return rv;
 }

(Note: tpmopen needs "int rv;" added to its locals; tpmclose keeps its
existing body with the flag clear moved under the lock.)