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 Tue, Jul 07, 2026 at 10:20:23PM +0200, Johann Höpfner wrote:
> > 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.
This is not MP related bug. Uniprocessor machines are also affected with
context switch issues.
> 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.
>
This is because lptpushbytes() could decrease sc->sc_count while is it
null.
> > 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 hunk fills the heap allocate buffer with null. In combination with
the "sc->sc_count == 0" check lpt(4) can't read outside this allocated
buffer. The output will be nulls or the data from another lpt(4)
threads.
>
> > 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
>
lpt(4) is kernel locked, so it works not as you described.
Case 1, T1 goes to sleep, T0 works without sleep, T1 performs integer
overflow:
T1: lptwrite sc_cp = sc_inbuf
sc_count = 1024
T1: lptpushbytes
context switch
----------------------------------------------
T0: lptwrite sc_cp = sc_inbuf
sc_count = 10
T0: lptpushbytes
sc_cp = sc_inbuf + 1
sc_count = 9
sc_cp = sc_inbuf + 2
sc_count = 8
....
sc_cp = sc_inbuf + 10
sc_count = 0
----------------------------------------------
T1: lptpushbytes continue run
sc_cp = sc_inbuf + 11
sc_count = SIZE_MAX
Case 2, T1 performs some iterations and goes to sleep, T0 performs some
iterations and goes to sleep, T1 continues.
T1: lptwrite sc_cp = sc_inbuf
sc_count = 10
T1: lptpushbytes
sc_cp = sc_inbuf + 1
sc_count = 9
sc_cp = sc_inbuf + 2
sc_count = 8
context switch
----------------------------------------------
T0: lptwrite sc_cp = sc_inbuf
sc_count = 20
T0: lptpushbytes
sc_cp = sc_inbuf + 1
sc_count = 19
sc_cp = sc_inbuf + 2
sc_count = 18
sc_cp = sc_inbuf + 3
sc_count = 17
sc_cp = sc_inbuf + 4
sc_count = 16
context switch
----------------------------------------------
T1: lptpushbytes
sc_cp = sc_inbuf + 4
sc_count = 16
sc_cp = sc_inbuf + 5
sc_count = 15
...
sc_cp = sc_inbuf + 20
sc_count = 0
----------------------------------------------
T0: lptpushbytes continue run
sc_cp = sc_inbuf + 21
sc_count = SIZE_MAX
> 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?
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.
>
> 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.
>
Again, this is not MP related issue.
>
> > 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.
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