Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
From: Li Nan
Date: Tue Jan 20 2026 - 02:55:58 EST
在 2026/1/16 22:39, Greg KH 写道:
On Thu, Jan 08, 2026 at 10:41:55AM +0800, linan666@xxxxxxxxxxxxxxx wrote:
From: Li Nan <linan122@xxxxxxxxxx>
NULL pointer dereference occurs when accessing 'port->physport->cad'
as below:
KASAN: null-ptr-deref in range [0x00000000000003f0-0x00000000000003f7]
RIP: 0010:parport_wait_peripheral+0x130/0x4b0
Call Trace:
parport_ieee1284_write_compat+0x306/0xb70
? __pfx_parport_ieee1284_write_compat+0x10/0x10
parport_write+0x1d6/0x660
lp_write+0x43e/0xbc0
? __pfx_lp_write+0x10/0x10
vfs_write+0x21c/0x960
ksys_write+0x12e/0x260
? __pfx_ksys_write+0x10/0x10
? __audit_syscall_entry+0x39e/0x510
do_syscall_64+0x59/0x110
entry_SYSCALL_64_after_hwframe+0x78/0xe2
The root cause is other processes may set 'port->cad' to NULL during
lp_write() operations. Process flow:
T1 T2
lp_write
lock port_mutex *
lp_claim_parport_or_block
parport_claim
port->cad = dev;
parport_write
parport_ieee1284_write_compat
lp_do_ioctl
lp_reset
lp_release_parport
parport_release
port->cad = NULL;
parport_wait_peripheral
port->physport->cad->timeout
|
NULL
Fix this issue by adding 'port_mutex' protection. Like read/write and
ioctl LPGETSTATUS, use this lock to protect port access and modification
to prevent concurrency problems.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
---
v2: Use mutex_lock instead of mutex_lock_interruptible in lp_release().
drivers/char/lp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 24417a00dfe9..82f2405b4502 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -520,9 +520,14 @@ static int lp_open(struct inode *inode, struct file *file)
should most likely only ever be used by the tunelp application. */
if ((LP_F(minor) & LP_ABORTOPEN) && !(file->f_flags & O_NONBLOCK)) {
int status;
+ if (mutex_lock_interruptible(&lp_table[minor].port_mutex)) {
+ ret = -EINTR;
+ goto out;
+ }
lp_claim_parport_or_block(&lp_table[minor]);
status = r_str(minor);
lp_release_parport(&lp_table[minor]);
+ mutex_unlock(&lp_table[minor].port_mutex);
if (status & LP_POUTPA) {
printk(KERN_INFO "lp%d out of paper\n", minor);
LP_F(minor) &= ~LP_BUSY;
@@ -547,6 +552,10 @@ static int lp_open(struct inode *inode, struct file *file)
goto out;
}
/* Determine if the peripheral supports ECP mode */
+ if (mutex_lock_interruptible(&lp_table[minor].port_mutex)) {
+ ret = -EINTR;
+ goto out;
+ }
lp_claim_parport_or_block(&lp_table[minor]);
if ((lp_table[minor].dev->port->modes & PARPORT_MODE_ECP) &&
!parport_negotiate(lp_table[minor].dev->port,
@@ -559,6 +568,7 @@ static int lp_open(struct inode *inode, struct file *file)
/* Leave peripheral in compatibility mode */
parport_negotiate(lp_table[minor].dev->port, IEEE1284_MODE_COMPAT);
lp_release_parport(&lp_table[minor]);
+ mutex_unlock(&lp_table[minor].port_mutex);
lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
out:
mutex_unlock(&lp_mutex);
@@ -569,10 +579,13 @@ static int lp_release(struct inode *inode, struct file *file)
{
unsigned int minor = iminor(inode);
+ /* ->release should never fail. Use uninterruptible mutex variant */
+ mutex_lock(&lp_table[minor].port_mutex);
But could stall for a very long time, right? That's not a good idea if
possible.
Thanks for the reply.
Yes, after looking deeper into the code, 'LP_F' prevents the same port from
being opened multiple times, and both open() and ioctl() are protected by
'lp_mutex'. So holding 'lp_mutex' in release() is more appropriate.
I will send a v3 to fix this soon.
And how was this tested?
We found this issue during fuzzing in QEMU.
Based on the root cause, I got the following reproducer:
```
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <pthread.h>
#include <sys/ioctl.h>
#include <string.h>
#include <errno.h>
#include <stdint.h>
#ifndef LPRESET
#define LPRESET 0x060c
#endif
static const char *devpath = "/dev/lp0";
static int fd = -1;
static volatile int stop = 0;
void *writer_thread(void *arg)
{
const size_t BUFSZ = 64 * 1024;
void *buf = malloc(BUFSZ);
if (!buf)
return NULL;
memset(buf, 0x55, BUFSZ);
printf("[writer] start writing loop to %s\n", devpath);
while (!stop) {
ssize_t w = write(fd, buf, BUFSZ);
if (w < 0) {
if (errno == EINTR)
continue;
perror("[writer] write");
break;
}
usleep(2000);
}
free(buf);
printf("[writer] exiting\n");
return NULL;
}
void *resetter_thread(void *arg)
{
printf("[resetter] will issue LPRESET ioctl in a loop\n");
// Wait a moment so writer likely enters blocking area
usleep(50000);
while (!stop) {
int r = ioctl(fd, LPRESET);
if (r < 0) {
if (errno == EINTR)
continue;
fprintf(stderr, "[resetter] ioctl(LPRESET) failed: %s\n", strerror(errno));
}
// small sleep to vary race window; tune this for your environment
usleep(1000);
}
printf("[resetter] exiting\n");
return NULL;
}
int main(int argc, char **argv)
{
if (argc >= 2) devpath = argv[1];
printf("repro_lp_race: device=%s\n", devpath);
fd = open(devpath, O_RDWR);
if (fd < 0) {
perror("open");
fprintf(stderr, "Try: sudo modprobe parport && sudo modprobe parport_pc && sudo modprobe lp\n");
return 1;
}
pthread_t wthr, rthr;
if (pthread_create(&wthr, NULL, writer_thread, NULL) != 0) {
perror("pthread_create writer");
close(fd);
return 1;
}
if (pthread_create(&rthr, NULL, resetter_thread, NULL) != 0) {
perror("pthread_create resetter");
stop = 1;
pthread_join(wthr, NULL);
close(fd);
return 1;
}
sleep(20);
stop = 1;
pthread_join(wthr, NULL);
pthread_join(rthr, NULL);
close(fd);
printf("done\n");
return 0;
}
```
thanks,
greg k-h
--
Thanks,
Nan