Download raw body.
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer
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 *);
sys/dev/ic/lpt.c: race condition, kernel heap leaked to line printer