[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fd9a77f5-8ed0-25d9-7195-445abe521a02@huaweicloud.com>
Date: Tue, 20 Jan 2026 15:55:47 +0800
From: Li Nan <linan666@...weicloud.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: arnd@...db.de, linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk,
wanghai38@...wei.com
Subject: Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
在 2026/1/16 22:39, Greg KH 写道:
> On Thu, Jan 08, 2026 at 10:41:55AM +0800, linan666@...weicloud.com wrote:
>> From: Li Nan <linan122@...wei.com>
>>
>> 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@...wei.com>
>> ---
>> 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
Powered by blists - more mailing lists