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: Tue, 7 Jul 2026 22:20:23 +0200 > You are wrong. During the lptwrite() race, the winner lptpushbytes() > or lptwrite() thread could set 'sc->sc_count' to 0. A few things first. I have only tested this on multiprocessor systems so that might be a hard requirement to even cause the bug. Secondly I only became aware of this due to the ASAN violation which it triggers (and would not if this was not an overflowed read) and have verified in a debugger, that yes, it sometimes accesses hundreds of bytes above the sc->sc_inbuf, and yes the data output to the parallel port matches exactly what is stored above sc_inbuf in the heap. > Index: sys/dev/ic/lpt.c > =================================================================== > RCS file: /cvs/src/sys/dev/ic/lpt.c,v > 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 7 Jul 2026 15:06:05 -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; This patch unfortunately does not help with my issue in any way, kernel heap continues to be sent over the parallel port. > This should not be the problem. In any case, the 'sc->sc_count' does not > exceed LPT_BSIZE bytes, so we never read outside 'sc->sc_inbuf'. My core disagrement with your assessment is sc->sc_count <= LPT_BSIZE is not sufficient for preventing BOs. A stronger, necessary condition is that sc->sc_cp + sc->sc_count <= sc->sc_inbuf + LPT_BSIZE I might need to explain the behavior I observed in debugging in a bit more detail. Hereafter follows one possible interleaving of two concurrent lptwrite execution, causing an overflow. T1: lptwrite sc_cp = sc_inbuf sc_count = 1024 ---------------------------------------------- T0: lptwrite: sc_cp = sc_inbuf ---------------------------------------------- T1: lptpushbytes sc_cp = sc_inbuf + 1 sc_count = 1023 ---------------------------------------------- T0: lptwrite sc_count = 1024 ---------------------------------------------- T1: lptpushbytes sc_cp = sc_inbuf + 2 sc_count = 1023 and as easy as this we gain one extra byte (really any number we like depending on the interleaving) of data being sent to the parallel port. Honestly I do not see a world where it could be safe to have one thread modifying sc->sc_cp while another is moving said pointer forward to copy memory it points at. Why would we not want to serialize the lptwrite's? Also, since sc_inbuf is shared, won't concurrent writes corrupt each other's data in any case? This whole driver seems to be designed without consideration of multiprocessing. It is more than a decade my senior and was likely just fine in a serial world. Fun fact: NetBSD has this exact bug too, as I found out this afternoon. > We do 'sc->sc_inbuf' allocation without M_ZERO flag, so after the device was > opened, we could deliver some heap data until 'sc->sc_inbuf' filled. But > this will be always the same data. Zeroing 'sc->sc_inbuf' should help. In testing, yes there is a lot of repeated output but that should be more or less expected as the sc_inbuf is fixed in memory for the lifetime of the file descriptor. And no, the heap data is not only delivered to the parallel port until the buffer is filled. I had the reproducer running for an hour today, continously spewing heap.