drivers/char/lp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
From: Li Nan <linan122@huawei.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@huawei.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);
lp_claim_parport_or_block(&lp_table[minor]);
parport_negotiate(lp_table[minor].dev->port, IEEE1284_MODE_COMPAT);
lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
lp_release_parport(&lp_table[minor]);
+ mutex_unlock(&lp_table[minor].port_mutex);
kfree(lp_table[minor].lp_buffer);
lp_table[minor].lp_buffer = NULL;
LP_F(minor) &= ~LP_BUSY;
@@ -641,7 +654,10 @@ static int lp_do_ioctl(unsigned int minor, unsigned int cmd,
return -EFAULT;
break;
case LPRESET:
+ if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
+ return -EINTR;
lp_reset(minor);
+ mutex_unlock(&lp_table[minor].port_mutex);
break;
#ifdef LP_STATS
case LPGETSTATS:
--
2.39.2
On Thu, Jan 08, 2026 at 10:41:55AM +0800, linan666@huaweicloud.com wrote:
> From: Li Nan <linan122@huawei.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@huawei.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.
And how was this tested?
thanks,
greg k-h
在 2026/1/16 22:39, Greg KH 写道:
> On Thu, Jan 08, 2026 at 10:41:55AM +0800, linan666@huaweicloud.com wrote:
>> From: Li Nan <linan122@huawei.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@huawei.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
On Tue, Jan 20, 2026 at 03:55:47PM +0800, Li Nan wrote: > > And how was this tested? > > We found this issue during fuzzing in QEMU. > Based on the root cause, I got the following reproducer: QEMU is not real hardware :) Do you really still use parallel port hardware in the real world? or is this just an accidemic exercise of a fuzzing tool? thanks, greg k-h
在 2026/1/20 17:23, Greg KH 写道: > On Tue, Jan 20, 2026 at 03:55:47PM +0800, Li Nan wrote: >>> And how was this tested? >> >> We found this issue during fuzzing in QEMU. >> Based on the root cause, I got the following reproducer: > > QEMU is not real hardware :) > > Do you really still use parallel port hardware in the real world? or > is this just an accidemic exercise of a fuzzing tool? > Yes, the issue was first found in QEMU. We do have some parallel port hardware for testing, as this module was used by our customers in the past, though we are not sure if it is still in active use today. This work is mainly part of routine maintenance. > thanks, > > greg k-h -- Thanks, Nan
On Tue, Jan 20, 2026 at 07:41:10PM +0800, Li Nan wrote: > > > 在 2026/1/20 17:23, Greg KH 写道: > > On Tue, Jan 20, 2026 at 03:55:47PM +0800, Li Nan wrote: > > > > And how was this tested? > > > > > > We found this issue during fuzzing in QEMU. > > > Based on the root cause, I got the following reproducer: > > > > QEMU is not real hardware :) > > > > Do you really still use parallel port hardware in the real world? or > > is this just an accidemic exercise of a fuzzing tool? > > > > Yes, the issue was first found in QEMU. > > We do have some parallel port hardware for testing, as this module was > used by our customers in the past, though we are not sure if it is still > in active use today. This work is mainly part of routine maintenance. Look at the big: /* !!! LOCKING IS NEEDED HERE */ in parport.c for a hint of maybe what you should be doing here instead. Your test example says you are allowing multiple users to access the parport at the same time, which would trigger this problem. But in the "real world" that isn't what happens as multiple parport accesses is not anything that is expected to work well, if at all (the same for serial ports). Also remember that this driver was written in the single processor days, so any locking was added much later, and odds are, is probably insuffient, to handle multiple accesses. I'd just prevent this from happening entirely by not letting your userspace to do this, to make things much simpler overall. So while I understand the fuzzing of this is "fun", the real-world applicability of attempting to add proper multi-user locking to the parport subsystem might not be a viable thing in the end, as I doubt the existing users of it need that type of thing. And the real-world users of this hardware probably are living just fine without it :) thanks, greg k-h
在 2026/1/20 20:43, Greg KH 写道: > On Tue, Jan 20, 2026 at 07:41:10PM +0800, Li Nan wrote: >> >> >> 在 2026/1/20 17:23, Greg KH 写道: >>> On Tue, Jan 20, 2026 at 03:55:47PM +0800, Li Nan wrote: >>>>> And how was this tested? >>>> >>>> We found this issue during fuzzing in QEMU. >>>> Based on the root cause, I got the following reproducer: >>> >>> QEMU is not real hardware :) >>> >>> Do you really still use parallel port hardware in the real world? or >>> is this just an accidemic exercise of a fuzzing tool? >>> >> >> Yes, the issue was first found in QEMU. >> >> We do have some parallel port hardware for testing, as this module was >> used by our customers in the past, though we are not sure if it is still >> in active use today. This work is mainly part of routine maintenance. > > Look at the big: > /* !!! LOCKING IS NEEDED HERE */ > in parport.c for a hint of maybe what you should be doing here instead. > > Your test example says you are allowing multiple users to access the > parport at the same time, which would trigger this problem. But in the > "real world" that isn't what happens as multiple parport accesses is not > anything that is expected to work well, if at all (the same for serial > ports). > > Also remember that this driver was written in the single processor days, > so any locking was added much later, and odds are, is probably > insuffient, to handle multiple accesses. I'd just prevent this from > happening entirely by not letting your userspace to do this, to make > things much simpler overall. > > So while I understand the fuzzing of this is "fun", the real-world > applicability of attempting to add proper multi-user locking to the > parport subsystem might not be a viable thing in the end, as I doubt the > existing users of it need that type of thing. And the real-world users > of this hardware probably are living just fine without it :) > > thanks, > > greg k-h Hi Greg, Thanks for your patient reply. Yes, code analysis shows concurrent access to this driver has many issues, and no problem reports means users don't perform such operations. Do you think we need cleanups/fixes in this driver, or should we wait for real-world users to encounter issues and discuss them separately? -- Thanks, Nan
On Tue, Jan 20, 2026 at 09:21:11PM +0800, Li Nan wrote: > Do you think we need cleanups/fixes in this driver, or should we wait > for real-world users to encounter issues and discuss them separately? As no one who actually has this hardware has reported anything in the decades that the code has been like this, I would recommend just leaving it as-is. thanks, greg k-h
在 2026/1/20 22:07, Greg KH 写道: > On Tue, Jan 20, 2026 at 09:21:11PM +0800, Li Nan wrote: >> Do you think we need cleanups/fixes in this driver, or should we wait >> for real-world users to encounter issues and discuss them separately? > > As no one who actually has this hardware has reported anything in the > decades that the code has been like this, I would recommend just leaving > it as-is. > > thanks, > > greg k-h Agreed. This patch can be ignored. :) -- Thanks, Nan
© 2016 - 2026 Red Hat, Inc.