> Date: Wed, 16 Sep 2026 08:28:17 +0200 > From: Johann Höpfner <[email protected]> > > A bug that I recently found in OpenBSD sys/dev/ic/lpt.c applies to > NetBSD and FreeBSD too. > > In short lptwrite reuses the same state struct under concurrent write > syscalls. [...]
Thanks, I filed a problem report in gnats to track this: PR kern/60760: lpt(4) concurrency issues https://gnats.NetBSD.org/60760 (Followups to this message, to [email protected] with subject line `Re: kern/60760: lpt(4) concurrency issues', will get appended to the PR, and if you want, I can add you to the notify-list.) > My prefered fix would be to only keep iterator context per-syscall > execution and not use the per-fd struct lpt_softc *sc there, but the > solution we settled on in OpenBSD was to simply initialize the buffer > and make sure that we only read from inside it. The output may still > skip over bytes or output zeros on concurrent writes to the same fd, but > wont impact the kernel. It's not clear to me that there is any value in allowing concurrent lptwrite calls at all. So rather than try to mitigate the damage of interleaving internal data structure updates like that, I'm inclined to just set a flag sc->sc_state |= LPT_WRITING while the first lptwrite is in progress, to block subsequent attempts to lptwrite until the first one is done. There are other issues with the driver too: in thread context, the state (sc_state, sc_count, sc_cp, &c.) is not protected from changes by the interrupt handler, so it likely needs various spltty/splx sprinkled in too. (Ideally, we would rewrite this driver with mutex/condvar(9) and make it MP-safe, but that involves some nontrivial testing on ancient platforms -- this approach of just sprinkling some spltty/splx and a lock flag is much lower-risk.) Draft attached, compile-tested only so far.
# HG changeset patch # User Taylor R Campbell <[email protected]> # Date 1790029347 0 # Mon Sep 21 22:22:27 2026 +0000 # Branch trunk # Node ID de49d4446a053906cf48f54d3855d454b392873f # Parent c96fe1ab6a49c6b24bccc49f7ccb16b325ab0468 # EXP-Topic riastradh-pr60760-lptfixes WIP: lpt(4): Fix concurrency and interrupt safety issues. PR kern/60760: lpt(4) concurrency issues diff -r c96fe1ab6a49 -r de49d4446a05 sys/dev/ic/lpt.c --- a/sys/dev/ic/lpt.c Sun Sep 20 03:58:19 2026 +0000 +++ b/sys/dev/ic/lpt.c Mon Sep 21 22:22:27 2026 +0000 @@ -158,6 +158,7 @@ lptopen(dev_t dev, int flag, int mode, s u_char control; int error; int spin; + int s; sc = device_lookup_private(&lpt_cd, LPTUNIT(dev)); if (!sc || !sc->sc_dev_ok) @@ -174,8 +175,12 @@ lptopen(dev_t dev, int flag, int mode, s sc->sc_state); #endif - if (sc->sc_state) - return EBUSY; + s = spltty(); + + if (sc->sc_state) { + error = EBUSY; + goto out; + } sc->sc_state = LPT_INIT; sc->sc_flags = flags; @@ -197,14 +202,15 @@ lptopen(dev_t dev, int flag, int mode, s for (spin = 0; NOT_READY_ERR(); spin += STEP) { if (spin >= TIMEOUT) { sc->sc_state = 0; - return EBUSY; + error = EBUSY; + goto out; } /* wait 1/4 second, give up if we get a signal */ error = tsleep((void *)sc, LPTPRI | PCATCH, "lptopen", STEP); if (error != EWOULDBLOCK) { sc->sc_state = 0; - return error; + goto out; } } @@ -216,6 +222,7 @@ lptopen(dev_t dev, int flag, int mode, s bus_space_write_1(iot, ioh, lpt_control, control); sc->sc_inbuf = malloc(LPT_BSIZE, M_DEVBUF, M_WAITOK); + sc->sc_cp = NULL; sc->sc_count = 0; sc->sc_state = LPT_OPEN; @@ -223,7 +230,10 @@ lptopen(dev_t dev, int flag, int mode, s lptwakeup(sc); LPRINTF(("%s: opened\n", device_xname(sc->sc_dev))); - return 0; + error = 0; + +out: splx(s); + return error; } int @@ -274,9 +284,12 @@ lptclose(dev_t dev, int flag, int mode, device_lookup_private(&lpt_cd, LPTUNIT(dev)); bus_space_tag_t iot = sc->sc_iot; bus_space_handle_t ioh = sc->sc_ioh; + int s; + s = spltty(); if (sc->sc_count) (void) lptpushbytes(sc); + sc->sc_count = 0; if ((sc->sc_flags & LPT_NOINTR) == 0) callout_stop(&sc->sc_wakeup_ch); @@ -285,6 +298,8 @@ lptclose(dev_t dev, int flag, int mode, sc->sc_state = 0; bus_space_write_1(iot, ioh, lpt_control, LPC_NINIT); free(sc->sc_inbuf, M_DEVBUF); + sc->sc_cp = sc->sc_inbuf = NULL; + splx(s); LPRINTF(("%s: closed\n", device_xname(sc->sc_dev))); return 0; @@ -367,8 +382,19 @@ lptwrite(dev_t dev, struct uio *uio, int struct lpt_softc *sc = device_lookup_private(&lpt_cd, LPTUNIT(dev)); size_t n; + int s; + bool locked = false; int error = 0; + s = spltty(); + while (sc->sc_state & LPT_WRITING) { + error = tsleep(&sc->sc_state, LPTPRI | PCATCH, "lptwrite", 0); + if (error) + goto out; + } + sc->sc_state |= LPT_WRITING; + locked = true; + while ((n = uimin(LPT_BSIZE, uio->uio_resid)) != 0) { uiomove(sc->sc_cp = sc->sc_inbuf, n, uio); sc->sc_count = n; @@ -380,9 +406,16 @@ lptwrite(dev_t dev, struct uio *uio, int */ uio->uio_resid += sc->sc_count; sc->sc_count = 0; - return error; + goto out; } } + error = 0; + +out: if (locked) { + sc->sc_state &= LPT_WRITING; + wakeup(&sc->sc_state); + } + splx(s); return 0; } diff -r c96fe1ab6a49 -r de49d4446a05 sys/dev/ic/lptvar.h --- a/sys/dev/ic/lptvar.h Sun Sep 20 03:58:19 2026 +0000 +++ b/sys/dev/ic/lptvar.h Mon Sep 21 22:22:27 2026 +0000 @@ -74,6 +74,7 @@ struct lpt_softc { #define LPT_OPEN 0x01 /* device is open */ #define LPT_OBUSY 0x02 /* printer is busy doing output */ #define LPT_INIT 0x04 /* waiting to initialize for open */ +#define LPT_WRITING 0x08 /* write in progress */ u_char sc_flags; #define LPT_AUTOLF 0x20 /* automatic LF on CR */ #define LPT_NOPRIME 0x40 /* don't prime on open */
