Re: sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer

On Tue, Jul 07, 2026 at 02:05:12PM +0200, Johann Höpfner wrote:
> On 26-07-07 09:51:33, Vitaliy Makkoveev wrote:
> > Hello Johann,
> > 
> > It seems we only need to check sc->sc_count against 0 after the sleep.
> > Otherwise the following "sc->sc_count--" produces integer overflow.
> > 
> > Does the diff below help?
> > 
> > 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 09:44:31 -0000
> > @@ -309,6 +309,8 @@ lptpushbytes(struct lpt_softc *sc)
> >  						error = EIO;
> >  					if (error != EWOULDBLOCK)
> >  						return error;
> > +					if (sc->sc_count == 0)
> > +						return 0;
> >  				}
> >  				break;
> >  			}
> 
> Hello Vitaliy.
> 
> I think this is unrelated. The original info leak was unaffected by
> your patch, when I tested it again.
>

You are wrong. During the lptwrite() race, the winner lptpushbytes()
or lptwrite() thread could set 'sc->sc_count' to 0.

> The race condition, as I understand it, is caused by sc_count being 
> effectively increased by one thread entering lptwrite while a second 
> thread is already executing lptpushbytes, now reading a wrong count of
> bytes left to be sent to the device (from sc->sc_count).

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'. 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.

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;