[PATCH v2] char: lp: Fix NULL pointer dereference of cad

linan666@huaweicloud.com posted 1 patch 1 month ago
drivers/char/lp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
[PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by linan666@huaweicloud.com 1 month ago
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
Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Greg KH 3 weeks, 1 day ago
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
Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Li Nan 2 weeks, 4 days ago

在 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

Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Greg KH 2 weeks, 4 days ago
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
Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Li Nan 2 weeks, 4 days ago

在 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

Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Greg KH 2 weeks, 4 days ago
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
Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Li Nan 2 weeks, 4 days ago

在 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

Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Greg KH 2 weeks, 4 days ago
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
Re: [PATCH v2] char: lp: Fix NULL pointer dereference of cad
Posted by Li Nan 2 weeks, 4 days ago

在 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