Download raw body.
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
On Wed, Jul 08, 2026 at 12:11:40PM +0200, Johann Höpfner wrote:
> > lpt(4) is kernel locked, so it works not as you described.
>
> Thanks for your reply. I read up on the locking code and now agree with
> your analysis and patches.
>
> However with both of your patches we still get kernel heap thrown onto
> the parallel port and a verified out of bounds read. I think, I know
> why.
>
> uiomove can block too. Thus in the current code Thread A could execute
> sc->sc_cp = sc->sc_inbuf in lptwrite; and then block in the uiomove. The
> sleep releases the kern_lock so that another thread could start
> executing lptwrite, then lptpushbytes and block while sc_cp > sc_inbuf.
> Thread A could wake back up and now only sets sc_count = n but does not
> reset sc_cp to the start of the buf leading to OOB reads.
>
Yes, userland buffer should be pushed to swap to trigger this.
> Only the following, third patch on top of your changes finally resolves
> this thing for me. You likely know more than me about the order these
> things should happen for error handling, but we definitely cant sleep in
> between setting sc_cp and sc_count.
>
>
> Except for this info leak, this is a matter of opinion about the
> guarantees userland should be provided, on which I will gladly defer to
> you.
>
In common case such guarantees are not possible unless you set single
thread.
Well, this is the final diff to fix this context switch fallout.
Index: sys/dev/ic/lpt.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/lpt.c,v
retrieving revision 1.17
diff -u -p -r1.17 lpt.c
--- sys/dev/ic/lpt.c 25 Jun 2025 20:28:09 -0000 1.17
+++ sys/dev/ic/lpt.c 8 Jul 2026 10:45:32 -0000
@@ -206,7 +206,7 @@ lptopen(dev_t dev, int flag, int mode, s
sc->sc_control = control;
bus_space_write_1(sc->sc_iot, sc->sc_ioh, lpt_control, control);
- sc->sc_inbuf = malloc(LPT_BSIZE, M_DEVBUF, M_WAITOK);
+ sc->sc_inbuf = malloc(LPT_BSIZE, M_DEVBUF, M_WAITOK | M_ZERO);
sc->sc_count = 0;
sc->sc_state = LPT_OPEN;
@@ -309,6 +309,8 @@ lptpushbytes(struct lpt_softc *sc)
error = EIO;
if (error != EWOULDBLOCK)
return error;
+ if (sc->sc_count == 0)
+ return 0;
}
break;
}
@@ -360,10 +362,10 @@ lptwrite(dev_t dev, struct uio *uio, int
int error = 0;
while ((n = ulmin(LPT_BSIZE, uio->uio_resid)) != 0) {
- sc->sc_cp = sc->sc_inbuf;
- error = uiomove(sc->sc_cp, n, uio);
+ error = uiomove(sc->sc_inbuf, n, uio);
if (error != 0)
return error;
+ sc->sc_cp = sc->sc_inbuf;
sc->sc_count = n;
error = lptpushbytes(sc);
if (error) {
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer