summaryrefslogtreecommitdiff
path: root/sys/dev/pad
diff options
context:
space:
mode:
authorriastradh <riastradh@NetBSD.org>2021-06-14 18:44:53 +0000
committerriastradh <riastradh@NetBSD.org>2021-06-14 18:44:53 +0000
commit69914541dc879bd98322ed0dc93e1db3951625f2 (patch)
tree41b9bae0f1682d932bf08d4182d99a9c83d2d2f7 /sys/dev/pad
parent9845ceebdbba4351475422b8b0a9e39427f029d6 (diff)
pad(4): Explain what's wrong with using device pointers like this.
...and why the kernel lock is not enough.
Diffstat (limited to 'sys/dev/pad')
-rw-r--r--sys/dev/pad/pad.c34
1 files changed, 32 insertions, 2 deletions
diff --git a/sys/dev/pad/pad.c b/sys/dev/pad/pad.c
index a0f2f20f1d1..fe0b429cf38 100644
--- a/sys/dev/pad/pad.c
+++ b/sys/dev/pad/pad.c
@@ -1,4 +1,4 @@
-/* $NetBSD: pad.c,v 1.74 2021/06/14 18:44:45 riastradh Exp $ */
+/* $NetBSD: pad.c,v 1.75 2021/06/14 18:44:53 riastradh Exp $ */
/*-
* Copyright (c) 2007 Jared D. McNeill <jmcneill@invisible.ca>
@@ -27,7 +27,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pad.c,v 1.74 2021/06/14 18:44:45 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pad.c,v 1.75 2021/06/14 18:44:53 riastradh Exp $");
#include <sys/param.h>
#include <sys/types.h>
@@ -386,6 +386,36 @@ pad_close(struct pad_softc *sc)
device_t self = sc->sc_dev;
cfdata_t cf = device_cfdata(self);
+ /*
+ * XXX This is not quite enough to prevent racing with drvctl
+ * detach. What can happen:
+ *
+ * cpu0 cpu1
+ *
+ * pad_close
+ * take kernel lock
+ * sc->sc_open = 0
+ * drop kernel lock
+ * wait for config_misc_lock
+ * drvctl detach
+ * take kernel lock
+ * drop kernel lock
+ * wait for config_misc_lock
+ * retake kernel lock
+ * drop config_misc_lock
+ * take config_misc_lock
+ * wait for kernel lock
+ * pad_detach (sc_open=0 already)
+ * free device
+ * drop kernel lock
+ * use device after free
+ *
+ * We need a way to grab a reference to the device so it won't
+ * be freed until we're done -- it's OK if we config_detach
+ * twice as long as it's idempotent, but not OK if the first
+ * config_detach frees the struct device before the second one
+ * has finished handling it.
+ */
KERNEL_LOCK(1, NULL);
KASSERT(sc->sc_open);
sc->sc_open = 0;