Re: [PATCH] ipack: ipoctal: fix use-after-free and null-ptr-deref on remove

[Pei Xiao]

在 2026/6/26 09:40, Shuangpeng 写道:
> Hi Pei,
>
> I applied the new patch and reran the same reproducer [1]. It triggers
> another KASAN report:
>
> [ 63.599954][ T1151] BUG: KASAN: slab-use-after-free in ipoctal_cleanup (include/linux/ipack.h:292 drivers/ipack/devices/ipoctal.c:697)
> [ 63.600513][ T1151] Read of size 8 at addr ffff8881136e5008 by task kworker/1:2/1151
> [ 63.601248][ T1151] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [ 63.601249][ T1151] Workqueue: events release_one_tty
> [ 63.601254][ T1151] Call Trace:
> [ 63.601257][ T1151]  <TASK>
> [ 63.601258][ T1151]  dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
> [ 63.601261][ T1151]  print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
> [ 63.601272][ T1151]  kasan_report (mm/kasan/report.c:595)
> [ 63.601286][ T1151]  ipoctal_cleanup (include/linux/ipack.h:292 drivers/ipack/devices/ipoctal.c:697)
> [ 63.601289][ T1151]  release_one_tty (drivers/tty/tty_io.c:1522)
> [ 63.601292][ T1151]  process_scheduled_works (kernel/workqueue.c:3322 kernel/workqueue.c:3405)
> [ 63.601297][ T1151]  worker_thread (kernel/workqueue.c:3486)
> [ 63.601303][ T1151]  kthread (kernel/kthread.c:436)
> [ 63.601310][ T1151]  ret_from_fork (kernel/process.c:158)
> [ 63.601320][ T1151]  ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
> [ 63.601324][ T1151]  </TASK>
> [ 63.610982][ T1151] The buggy address belongs to the object at ffff8881136e5000
> [ 63.610982][ T1151]  which belongs to the cache kmalloc-1k of size 1024
> [ 63.611938][ T1151] The buggy address is located 8 bytes inside of
> [ 63.611938][ T1151]  freed 1024-byte region [ffff8881136e5000, ffff8881136e5400)
>
> I am also happy to test the next version if needed.
You're really a warm-hearted member of the Linux kernel community.
thanks!~

> Best,
> Shuangpeng
>
> [1] https://gist.github.com/shuangpengbai/bc71ccd7ce1454abae45897fdde6813b
>
>
>> On Jun 24, 2026, at 23:24, Pei Xiao <xiaopei01@xxxxxxxxxx> wrote:
>>
>> A use-after-free occurs when the device is removed while a tty
>> session is still active.  The remove callback frees the ipoctal
>> structure via kfree() while tty ops may still access it.
>>
>> Fix this by introducing kref-based lifetime management. The kref is
>> initialized in probe(), taken in ipoctal_install() when a tty is
>> opened, and released in ipoctal_cleanup() when the tty is finally
>> destroyed.  The remove callback replaces direct kfree() with
>> kref_put(), ensuring the memory is freed only after all references
>> are released.
>>
>> However, the kref keeps only the ipoctal struct alive — the MMIO
>> mappings (devm_ioremap) and xmit_buf are released during remove()
>> regardless.  If a tty fd is still open:
>>
>>  - tty_port_free_xmit_buf() NULLs xmit_buf, leading to a NULL
>>    dereference in ipoctal_write_tty().
>>  - devm iounmap() removes page table mappings, leading to a page
>>    fault in ipoctal_set_termios() and other tty ops that access
>>    channel->regs via iowrite8().
>>
>> Fix both by:
>>
>>  (1) checking for NULL xmit_buf in ipoctal_write_tty(), and
>>  (2) introducing a "removed" flag in struct ipoctal, set at the
>>      start of __ipoctal_remove(), and checked in every tty op that
>>      touches hardware registers (write_tty, set_termios,
>>      port_activate, hangup, shutdown).
>>
>> Reported-by: Shuangpeng Bai <shuangpeng.kernel@xxxxxxxxx>
>> Closes: https://lore.kernel.org/lkml/178144969601.60470.1257088106279546587@xxxxxxxxx/
>> Fixes: 05e5027efc9c ("Staging: ipack: move out of staging")
>> Signed-off-by: Pei Xiao <xiaopei01@xxxxxxxxxx>
>> ---
>> drivers/ipack/devices/ipoctal.c | 45 +++++++++++++++++++++++++++++++--
>> 1 file changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
>> index 1bbefc6de708..2b9d381cf56d 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,8 @@ struct ipoctal {
>> struct tty_driver *tty_drv;
>> u8 __iomem *mem8_space;
>> u8 __iomem *int_space;
>> + struct kref kref;
>> + bool removed;
>> };
>>
>> static inline struct ipoctal *chan_to_ipoctal(struct ipoctal_channel *chan,
>> @@ -70,8 +75,13 @@ 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 +105,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 +471,12 @@ 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 +516,12 @@ 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 +650,15 @@ 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 +675,15 @@ 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);
>> }
>> @@ -666,6 +695,7 @@ static void ipoctal_cleanup(struct tty_struct *tty)
>>
>> /* release the carrier driver */
>> ipack_put_carrier(ipoctal->dev);
>> + kref_put(&ipoctal->kref, ipoctal_release);
>> }
>>
>> static const struct tty_operations ipoctal_fops = {
>> @@ -683,6 +713,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,6 +729,8 @@ static int ipoctal_probe(struct ipack_device *dev)
>> if (ipoctal == NULL)
>> return -ENOMEM;
>>
>> + kref_init(&ipoctal->kref);
>> +
>> ipoctal->dev = dev;
>> res = ipoctal_inst_slot(ipoctal, dev->bus->bus_nr, dev->slot);
>> if (res)
>> @@ -701,7 +740,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 +748,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 +766,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
>>