[PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[Pei Xiao]

When the ipoctal device is removed while a userspace process still
holds a tty fd open, several races and use-after-free bugs can be
triggered.  This series addresses the following issues:

1. UAF of struct ipoctal: the remove callback freed ipoctal via
   kfree() while in-flight tty ops could still be accessing it.

2. NULL dereference in ipoctal_write_tty(): __ipoctal_remove() freed
   xmit_buf while a concurrent write() could still dereference it.

3. UAF in ipoctal_cleanup(): ipack_put_carrier(ipoctal->dev) walked
   through ipoctal->dev to reach the carrier module, but the
   ipack_device could have already been freed by then.

4. Page faults due to devm_ioremap() regions being unmapped while
   tty ops still access hardware registers.

5. TOCTOU race between the removed-flag check and the subsequent
   hardware access in each tty op.

Patch 1 introduces kref-based lifetime management, caches the
carrier owner module pointer to avoid chasing a dangling dev->bus
pointer, adds a "removed" flag with checks in all tty ops that
touch hardware, and adds a NULL guard on xmit_buf in the write path.

Patch 2 adds a read-write semaphore (remove_sem) to close the
TOCTOU window: the tty ops hold the read lock for the duration of
the hardware access, while __ipoctal_remove() acquires the write
lock when setting the removed flag, ensuring that once removed is
true no in-flight tty op is still inside a critical section.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Link: https://lore.kernel.org/lkml/178144969601.60470.1257088106279546587@gmail.com/

Pei Xiao (2):
  ipack: ipoctal: fix UAF, null-ptr-deref, and use-after-free in cleanup
    on remove
  ipack: ipoctal: add rwsem to guard against TOCTOU in remove path

 drivers/ipack/devices/ipoctal.c | 60 ++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

--
2.25.1

[PATCH 1/2] ipack: ipoctal: fix UAF, null-ptr-deref, and use-after-free in cleanup on remove

[Pei Xiao]

Three issues arise when the device is removed while a tty session is
still active:

1. UAF of struct ipoctal: the remove callback frees ipoctal via
   kfree() while tty ops may still access it.  Fix by introducing
   kref-based lifetime management — kref is taken in install() when
   a tty is opened and released in cleanup() when the tty is finally
   destroyed; remove() uses kref_put() instead of kfree().

2. NULL dereference in ipoctal_write_tty(): __ipoctal_remove()
   frees xmit_buf via tty_port_free_xmit_buf() while a userspace
   process may still hold the tty fd and call write().  Fix by
   checking for NULL xmit_buf in ipoctal_write_tty().

3. UAF in ipoctal_cleanup(): ipack_put_carrier(ipoctal->dev)
   dereferences ipoctal->dev after the ipack_device has been freed
   by ipack_device_del().  Fix by caching ipoctal->carrier_owner
   during probe() and calling module_put() on the cached pointer
   directly in cleanup(), avoiding any access to ipoctal->dev.

Also introduce a "removed" flag in struct ipoctal, set at the start
of __ipoctal_remove(), and checked in every tty op that accesses
hardware resources (port_activate, write_tty, set_termios, hangup,
shutdown).  This prevents page faults when devm_ioremap() regions
are unmapped after remove() returns.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/lkml/178144969601.60470.1257088106279546587@gmail.com/
Fixes: 05e5027efc9c ("Staging: ipack: move out of staging")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/ipack/devices/ipoctal.c | 56 ++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index 1bbefc6de708..bf71b8952a7c 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -10,6 +10,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/kref.h>
 #include <linux/sched.h>
 #include <linux/tty.h>
 #include <linux/serial.h>
@@ -25,6 +26,8 @@
 
 static const struct tty_operations ipoctal_fops;
 
+static void ipoctal_release(struct kref *kref);
+
 struct ipoctal_channel {
 	struct ipoctal_stats		stats;
 	unsigned int			nb_bytes;
@@ -49,6 +52,9 @@ struct ipoctal {
 	struct tty_driver		*tty_drv;
 	u8 __iomem			*mem8_space;
 	u8 __iomem			*int_space;
+	struct kref			kref;
+	struct module			*carrier_owner;
+	bool				removed;
 };
 
 static inline struct ipoctal *chan_to_ipoctal(struct ipoctal_channel *chan,
@@ -70,8 +76,14 @@ static void ipoctal_reset_channel(struct ipoctal_channel *channel)
 static int ipoctal_port_activate(struct tty_port *port, struct tty_struct *tty)
 {
 	struct ipoctal_channel *channel;
+	struct ipoctal *ipoctal;
 
 	channel = dev_get_drvdata(tty->dev);
+	ipoctal = chan_to_ipoctal(channel, tty->index);
+
+
+	if (ipoctal->removed)
+		return -ENODEV;
 
 	/*
 	 * Enable RX. TX will be enabled when
@@ -95,6 +107,7 @@ static int ipoctal_install(struct tty_driver *driver, struct tty_struct *tty)
 	if (res)
 		goto err_put_carrier;
 
+	kref_get(&ipoctal->kref);
 	tty->driver_data = channel;
 
 	return 0;
@@ -460,8 +473,13 @@ static ssize_t ipoctal_write_tty(struct tty_struct *tty, const u8 *buf,
 				 size_t count)
 {
 	struct ipoctal_channel *channel = tty->driver_data;
+	struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty->index);
 	size_t char_copied;
 
+
+	if (ipoctal->removed || !channel->tty_port.xmit_buf)
+		return 0;
+
 	char_copied = ipoctal_copy_write_buffer(channel, buf, count);
 
 	/* As the IP-OCTAL 485 only supports half duplex, do it manually */
@@ -501,8 +519,13 @@ static void ipoctal_set_termios(struct tty_struct *tty,
 	unsigned char mr2 = 0;
 	unsigned char csr = 0;
 	struct ipoctal_channel *channel = tty->driver_data;
+	struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty->index);
 	speed_t baud;
 
+
+	if (ipoctal->removed)
+		return;
+
 	cflag = tty->termios.c_cflag;
 
 	/* Disable and reset everything before change the setup */
@@ -631,10 +654,16 @@ static void ipoctal_hangup(struct tty_struct *tty)
 {
 	unsigned long flags;
 	struct ipoctal_channel *channel = tty->driver_data;
+	struct ipoctal *ipoctal;
 
 	if (channel == NULL)
 		return;
 
+	ipoctal = chan_to_ipoctal(channel, tty->index);
+
+	if (ipoctal->removed)
+		return;
+
 	spin_lock_irqsave(&channel->lock, flags);
 	channel->nb_bytes = 0;
 	channel->pointer_read = 0;
@@ -651,10 +680,16 @@ static void ipoctal_hangup(struct tty_struct *tty)
 static void ipoctal_shutdown(struct tty_struct *tty)
 {
 	struct ipoctal_channel *channel = tty->driver_data;
+	struct ipoctal *ipoctal;
 
 	if (channel == NULL)
 		return;
 
+	ipoctal = chan_to_ipoctal(channel, tty->index);
+
+	if (ipoctal->removed)
+		return;
+
 	ipoctal_reset_channel(channel);
 	tty_port_set_initialized(&channel->tty_port, false);
 }
@@ -664,8 +699,9 @@ static void ipoctal_cleanup(struct tty_struct *tty)
 	struct ipoctal_channel *channel = tty->driver_data;
 	struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty->index);
 
-	/* release the carrier driver */
-	ipack_put_carrier(ipoctal->dev);
+	/* release the carrier driver via cached owner */
+	module_put(ipoctal->carrier_owner);
+	kref_put(&ipoctal->kref, ipoctal_release);
 }
 
 static const struct tty_operations ipoctal_fops = {
@@ -683,6 +719,13 @@ static const struct tty_operations ipoctal_fops = {
 	.cleanup =              ipoctal_cleanup,
 };
 
+static void ipoctal_release(struct kref *kref)
+{
+	struct ipoctal *ipoctal = container_of(kref, struct ipoctal, kref);
+
+	kfree(ipoctal);
+}
+
 static int ipoctal_probe(struct ipack_device *dev)
 {
 	int res;
@@ -692,7 +735,10 @@ static int ipoctal_probe(struct ipack_device *dev)
 	if (ipoctal == NULL)
 		return -ENOMEM;
 
+	kref_init(&ipoctal->kref);
+
 	ipoctal->dev = dev;
+	ipoctal->carrier_owner = dev->bus->owner;
 	res = ipoctal_inst_slot(ipoctal, dev->bus->bus_nr, dev->slot);
 	if (res)
 		goto out_uninst;
@@ -701,7 +747,7 @@ static int ipoctal_probe(struct ipack_device *dev)
 	return 0;
 
 out_uninst:
-	kfree(ipoctal);
+	kref_put(&ipoctal->kref, ipoctal_release);
 	return res;
 }
 
@@ -709,6 +755,8 @@ static void __ipoctal_remove(struct ipoctal *ipoctal)
 {
 	int i;
 
+	ipoctal->removed = true;
+
 	ipoctal->dev->bus->ops->free_irq(ipoctal->dev);
 
 	for (i = 0; i < NR_CHANNELS; i++) {
@@ -725,7 +773,7 @@ static void __ipoctal_remove(struct ipoctal *ipoctal)
 	tty_unregister_driver(ipoctal->tty_drv);
 	kfree(ipoctal->tty_drv->name);
 	tty_driver_kref_put(ipoctal->tty_drv);
-	kfree(ipoctal);
+	kref_put(&ipoctal->kref, ipoctal_release);
 }
 
 static void ipoctal_remove(struct ipack_device *idev)
-- 
2.25.1

[PATCH 2/2] ipack: ipoctal: add rwsem to guard against TOCTOU in remove path

[Pei Xiao]

The "removed" flag check in each tty op has a TOCTOU race with
__ipoctal_remove(): the device could be removed between the flag
check and the subsequent access to hardware resources (channel
registers via iowrite8, or xmit_buf in write_tty).

Close this race by introducing a read-write semaphore (remove_sem).
The tty ops acquire the read lock via guard(rwsem_read) for the
full duration of the operation, while __ipoctal_remove() acquires
the write lock via scoped_guard(rwsem_write) when setting the
removed flag.  This ensures that once removed is true, no in-flight
tty op can still be accessing resources that are about to be freed
by the remove path.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/ipack/devices/ipoctal.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index bf71b8952a7c..2169e4b75f98 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/kref.h>
+#include <linux/rwsem.h>
 #include <linux/sched.h>
 #include <linux/tty.h>
 #include <linux/serial.h>
@@ -54,6 +55,7 @@ struct ipoctal {
 	u8 __iomem			*int_space;
 	struct kref			kref;
 	struct module			*carrier_owner;
+	struct rw_semaphore		remove_sem;
 	bool				removed;
 };
 
@@ -81,7 +83,7 @@ static int ipoctal_port_activate(struct tty_port *port, struct tty_struct *tty)
 	channel = dev_get_drvdata(tty->dev);
 	ipoctal = chan_to_ipoctal(channel, tty->index);
 
-
+	guard(rwsem_read)(&ipoctal->remove_sem);
 	if (ipoctal->removed)
 		return -ENODEV;
 
@@ -476,7 +478,7 @@ static ssize_t ipoctal_write_tty(struct tty_struct *tty, const u8 *buf,
 	struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty->index);
 	size_t char_copied;
 
-
+	guard(rwsem_read)(&ipoctal->remove_sem);
 	if (ipoctal->removed || !channel->tty_port.xmit_buf)
 		return 0;
 
@@ -522,7 +524,7 @@ static void ipoctal_set_termios(struct tty_struct *tty,
 	struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty->index);
 	speed_t baud;
 
-
+	guard(rwsem_read)(&ipoctal->remove_sem);
 	if (ipoctal->removed)
 		return;
 
@@ -660,7 +662,7 @@ static void ipoctal_hangup(struct tty_struct *tty)
 		return;
 
 	ipoctal = chan_to_ipoctal(channel, tty->index);
-
+	guard(rwsem_read)(&ipoctal->remove_sem);
 	if (ipoctal->removed)
 		return;
 
@@ -686,7 +688,7 @@ static void ipoctal_shutdown(struct tty_struct *tty)
 		return;
 
 	ipoctal = chan_to_ipoctal(channel, tty->index);
-
+	guard(rwsem_read)(&ipoctal->remove_sem);
 	if (ipoctal->removed)
 		return;
 
@@ -736,6 +738,7 @@ static int ipoctal_probe(struct ipack_device *dev)
 		return -ENOMEM;
 
 	kref_init(&ipoctal->kref);
+	init_rwsem(&ipoctal->remove_sem);
 
 	ipoctal->dev = dev;
 	ipoctal->carrier_owner = dev->bus->owner;
@@ -755,7 +758,8 @@ static void __ipoctal_remove(struct ipoctal *ipoctal)
 {
 	int i;
 
-	ipoctal->removed = true;
+	scoped_guard(rwsem_write, &ipoctal->remove_sem)
+		ipoctal->removed = true;
 
 	ipoctal->dev->bus->ops->free_irq(ipoctal->dev);
 
-- 
2.25.1

Re: [PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[Greg KH]

On Wed, Jul 01, 2026 at 10:01:08AM +0800, Pei Xiao wrote:
> When the ipoctal device is removed while a userspace process still
> holds a tty fd open, several races and use-after-free bugs can be
> triggered.  This series addresses the following issues:

Cool, but why remove the module at all?

And do you have this hardware to test with?

thanks,

greg k-h

Re: [PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[Pei Xiao]

在 2026/7/1 13:41, Greg KH 写道:
> On Wed, Jul 01, 2026 at 10:01:08AM +0800, Pei Xiao wrote:
>> When the ipoctal device is removed while a userspace process still
>> holds a tty fd open, several races and use-after-free bugs can be
>> triggered.  This series addresses the following issues:
> Cool, but why remove the module at all?
1. Carrier hot-unplug: the IPack carrier (e.g., TPCI-200) is a PCI
   device.  When the carrier is physically removed or the PCI device
   is unbound via sysfs, ipack_bus_unregister() calls
   ipack_device_del() on every mezzanine device, which invokes
   ipoctal_remove().  This happens regardless of whether userspace
   still holds a tty fd open -- the carrier is gone, so the devices
   must go too.  The ipack_get_carrier()/try_module_get() in
   ipoctal_install() only prevents rmmod of the carrier module, not
   physical removal.

2. Syzkaller/syzbot-style fuzzing: the original report from
   Shuangpeng Bai  exercises exactly these paths.  Even without
   real hardware, syzkaller can trigger the races by binding and
   unbinding drivers.

So while rmmod of ipoctal itself is blocked by the tty layer (via
drv->owner), the remove callback can and will be called through
carrier removal or driver unbind.  The crashes are real.
> And do you have this hardware to test with?
I don't have the hardware.  However:

- The fixes are structural and follow well-established kernel
  patterns: kref for lifetime management, rwsem for TOCTOU races,
  and caching a module pointer to avoid chasing a freed bus pointer.
  These are the same idioms used across the tty layer and other
  subsystems.

The original reporter (Shuangpeng, CC'd) has tested this crash passed.

thanks for your responce and time!
Pei.
Thanks!
> thanks,
>
> greg k-h

Re: [PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[Greg KH]

On Wed, Jul 01, 2026 at 02:16:50PM +0800, Pei Xiao wrote:
> 
> 
> 在 2026/7/1 13:41, Greg KH 写道:
> > On Wed, Jul 01, 2026 at 10:01:08AM +0800, Pei Xiao wrote:
> >> When the ipoctal device is removed while a userspace process still
> >> holds a tty fd open, several races and use-after-free bugs can be
> >> triggered.  This series addresses the following issues:
> > Cool, but why remove the module at all?
> 1. Carrier hot-unplug: the IPack carrier (e.g., TPCI-200) is a PCI
>    device.  When the carrier is physically removed or the PCI device
>    is unbound via sysfs, ipack_bus_unregister() calls
>    ipack_device_del() on every mezzanine device, which invokes
>    ipoctal_remove().  This happens regardless of whether userspace
>    still holds a tty fd open -- the carrier is gone, so the devices
>    must go too.  The ipack_get_carrier()/try_module_get() in
>    ipoctal_install() only prevents rmmod of the carrier module, not
>    physical removal.

Yes, but does anyone actually remove the module or unbind it or remove
it physically in a real system?

This is very very old hardware, removing it while active is not
something that the hardware is designed to support, so why do we need to
support it in the kernel?

Do you have this hardware?

> 2. Syzkaller/syzbot-style fuzzing: the original report from
>    Shuangpeng Bai  exercises exactly these paths.  Even without
>    real hardware, syzkaller can trigger the races by binding and
>    unbinding drivers.

Sure, but remember bind/unbind is a DEBUG TOOL!  And it is controlled by
root, so it is a "best effort" thing that is there for kernel developers
to use for working on their code.  No real customer runs that code path
(and if they do, they are an admin and can do much worse things to the
system if they wanted to.)

> So while rmmod of ipoctal itself is blocked by the tty layer (via
> drv->owner), the remove callback can and will be called through
> carrier removal or driver unbind.  The crashes are real.

But not a real use case ;)

> > And do you have this hardware to test with?
> I don't have the hardware.  However:
> 
> - The fixes are structural and follow well-established kernel
>   patterns: kref for lifetime management, rwsem for TOCTOU races,
>   and caching a module pointer to avoid chasing a freed bus pointer.
>   These are the same idioms used across the tty layer and other
>   subsystems.
> 
> The original reporter (Shuangpeng, CC'd) has tested this crash passed.

With real hardware?

Ideally we could just remove the driver as I doubt anyone uses it
anymore.  You can't even buy the device.

thanks,

greg k-h

Re: [PATCH 0/2] ipack: ipoctal: fix races and UAFs during module removal

[Pei Xiao]

在 2026/7/1 18:37, Greg KH 写道:
> On Wed, Jul 01, 2026 at 02:16:50PM +0800, Pei Xiao wrote:
>>
>> 在 2026/7/1 13:41, Greg KH 写道:
>>> On Wed, Jul 01, 2026 at 10:01:08AM +0800, Pei Xiao wrote:
>>>> When the ipoctal device is removed while a userspace process still
>>>> holds a tty fd open, several races and use-after-free bugs can be
>>>> triggered.  This series addresses the following issues:
>>> Cool, but why remove the module at all?
>> 1. Carrier hot-unplug: the IPack carrier (e.g., TPCI-200) is a PCI
>>    device.  When the carrier is physically removed or the PCI device
>>    is unbound via sysfs, ipack_bus_unregister() calls
>>    ipack_device_del() on every mezzanine device, which invokes
>>    ipoctal_remove().  This happens regardless of whether userspace
>>    still holds a tty fd open -- the carrier is gone, so the devices
>>    must go too.  The ipack_get_carrier()/try_module_get() in
>>    ipoctal_install() only prevents rmmod of the carrier module, not
>>    physical removal.
> Yes, but does anyone actually remove the module or unbind it or remove
> it physically in a real system?
>
> This is very very old hardware, removing it while active is not
> something that the hardware is designed to support, so why do we need to
> support it in the kernel?
>
> Do you have this hardware?
>
>> 2. Syzkaller/syzbot-style fuzzing: the original report from
>>    Shuangpeng Bai  exercises exactly these paths.  Even without
>>    real hardware, syzkaller can trigger the races by binding and
>>    unbinding drivers.
> Sure, but remember bind/unbind is a DEBUG TOOL!  And it is controlled by
> root, so it is a "best effort" thing that is there for kernel developers
> to use for working on their code.  No real customer runs that code path
> (and if they do, they are an admin and can do much worse things to the
> system if they wanted to.)
>
>> So while rmmod of ipoctal itself is blocked by the tty layer (via
>> drv->owner), the remove callback can and will be called through
>> carrier removal or driver unbind.  The crashes are real.
> But not a real use case ;)
>
>>> And do you have this hardware to test with?
>> I don't have the hardware.  However:
>>
>> - The fixes are structural and follow well-established kernel
>>   patterns: kref for lifetime management, rwsem for TOCTOU races,
>>   and caching a module pointer to avoid chasing a freed bus pointer.
>>   These are the same idioms used across the tty layer and other
>>   subsystems.
>>
>> The original reporter (Shuangpeng, CC'd) has tested this crash passed.
> With real hardware?
>
> Ideally we could just remove the driver as I doubt anyone uses it
> anymore.  You can't even buy the device.
You are right, but I saw the phrase 'gladly review patches from others' and
 thought that although this is a very, very old driver, there might
still be a 
chance to get my changes merged. Haha, I really owe an apology to 
Shuangpeng, who tested my patches several times for me.

 Also, I may have wasted your time, given how busy you are.

Pei.
Thanks!
>
> thanks,
>
> greg k-h