From: Johann Höpfner Subject: Re: sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer To: Vitaliy Makkoveev Cc: tech@openbsd.org Date: Wed, 8 Jul 2026 12:11:40 +0200 > 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. 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. Index: sys/dev/ic/lpt.c =================================================================== --- sys/dev/ic/lpt.c +++ sys/dev/ic/lpt.c @@ -362,10 +362,10 @@ lptwrite(dev_t dev, struct uio *uio, int flags) 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) { > And so what? If userland is such stupid to perform non seralized writes > to lpt(4), should we prevent this? Also, in some cases there is > impossible to prevent this. 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.