Index | Thread | Search

From:
Johann Höpfner <hoepf@cit.tum.de>
Subject:
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
To:
tech@openbsd.org
Date:
Tue, 7 Jul 2026 01:03:51 +0200

Download raw body.

Thread
Hello tech@

Again in testing my experimental kernel-address-sanitizer in syzkaller 
another crash occurred that I believe to be a bug.

lptwrite, if executed on the same file concurrently has a race condition 
wherein sc->sc_cp, sc->sc_count, etc can be (re)set by one thread entering
lptwrite while another one executes lptpushbytes. This causes an out of 
bounds read against sc->sc_inbuf as the loop keeps advancing the location
that is read in the first which however reads an sc_count set by any 
later thread to execute the first part of lptwrite.

Steps to reproduce:

run openbsd in a way that allows you to attach and monitor from a parallel
port device, e.g. as under qemu using 

```
-chardev file,id=charptr0,path=log.txt \
-device isa-parallel,chardev=charptr0
```

then run the attached program as root (adjust the device if no output is
received by the virtual printer)

This bug has been quite unreliable for me, the output varying between 
<1% to 70% leaked heap memory depending on the host system, the rest being
the letter A as one would expected.

I would like to apologize in advance if reproducing this is painful.

```c
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <pthread.h>
#include <string.h>
#include <errno.h>
#include <time.h>
#include <stdint.h>
#include <sys/stat.h>

#define NUM_WRITERS 32
#define ITERATIONS 1000

int fd;

void *writer_thread(void *arg) {
    (void)arg;
    char buf[4096];
    srand((unsigned int)(uintptr_t)arg + time(NULL));

    for (int i = 0; i < ITERATIONS; i++) {
        size_t len = (rand() % 1024) + 1;
        memset(buf, 'A', len);

        ssize_t ret = write(fd, buf, len);
        if (ret < 0) {
            if (errno == EINTR || errno == EIO || errno == ENXIO || errno == EBUSY)
                continue;
            perror("write");
            break;
        }
    }
    return NULL;
}

int main(void) {
    fd = open("/dev/lpa0", O_WRONLY);
    if (fd < 0) {
        perror("open");
        return 1;
    }

    pthread_t writers[NUM_WRITERS];
    for (int i = 0; i < NUM_WRITERS; i++)
        pthread_create(&writers[i], NULL, writer_thread, (void *)(uintptr_t)i);

    for (int i = 0; i < NUM_WRITERS; i++)
        pthread_join(writers[i], NULL);

    return 0;
}
```

We expect only the byte 'A' to be output in repeat but get 'A's 
intermixed with leaked data from kernel heap allocations.

I propose the following patch serializing writes per port, unless someone 
more familiar with the lpt driver can offer a less aggressive version.

Index: sys/dev/ic/lpt.c
===================================================================
--- sys/dev/ic/lpt.c
+++ sys/dev/ic/lpt.c
@@ -356,14 +356,17 @@
 lptwrite(dev_t dev, struct uio *uio, int flags)
 {
 	struct lpt_softc *sc = lpt_cd.cd_devs[LPTUNIT(dev)];
+	rw_enter_write(&sc->sc_lock);
 	size_t n;
 	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);
-		if (error != 0)
+		if (error != 0) {
+			rw_exit_write(&sc->sc_lock);
 			return error;
+		}
 		sc->sc_count = n;
 		error = lptpushbytes(sc);
 		if (error) {
@@ -373,9 +376,11 @@ lptwrite(dev_t dev, struct uio *uio, int flags)
 			 */
 			uio->uio_resid += sc->sc_count;
 			sc->sc_count = 0;
+			rw_exit_write(&sc->sc_lock);
 			return error;
 		}
 	}
+	rw_exit_write(&sc->sc_lock);
 	return 0;
 }

Index: sys/dev/ic/lptvar.h
===================================================================
--- sys/dev/ic/lptvar.h
+++ sys/dev/ic/lptvar.h
@@ -74,6 +74,7 @@
 #define	LPT_NOINTR	0x80	/* do not use interrupt */
 	u_int8_t sc_control;
 	u_int8_t sc_laststatus;
+	struct rwlock sc_lock;
 };
 
 int	lptintr(void *);