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

[Pei Xiao]

在 2026/6/25 10:10, Shuangpeng 写道:
> Hi Pei,
>
> Thanks for the updated patch!
>
> I applied it and reran the same reproducer. The previous crash in
> ipoctal_write_tty() is no longer triggered, but the same reproducer now
> hits another Oops after removing the device while the tty fd is still open.
>
> The new report looks like this:
>
> BUG: unable to handle page fault for address: ffffc9000617b005
> #PF: supervisor write access in kernel mode
> Oops: Oops: 0002 [#1] SMP KASAN PTI
> RIP: 0010:iowrite8 (lib/iomap.c:207)
>
> Call Trace:
>   ipoctal_set_termios (drivers/ipack/devices/ipoctal.c:66 drivers/ipack/devices/ipoctal.c:517)
>   tty_set_termios (drivers/tty/tty_ioctl.c:341)
>   set_termios (drivers/tty/tty_ioctl.c:516)
>   tty_mode_ioctl (drivers/tty/tty_ioctl.c:?)
>   tty_ioctl (drivers/tty/tty_io.c:2801)
>   __se_sys_ioctl (fs/ioctl.c:51 fs/ioctl.c:597 fs/ioctl.c:583)
>   do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>
> The faulting access is from ipoctal_reset_channel():
>
> drivers/ipack/devices/ipoctal.c:66
> iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, &channel->regs->w.cr);
>
> called by ipoctal_set_termios():
>
> drivers/ipack/devices/ipoctal.c:517
> ipoctal_reset_channel(channel);
thanks again!
I have already send v3 for test!
>
> Best,
> Shuangpeng
>
>
>> On Jun 24, 2026, at 21:33, 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 ipoctal_write_tty() and other tty ops
>> may still access it.
>>
>> Fix the UAF by introducing kref-based lifetime management for the
>> ipoctal structure. A kref is taken in ipoctal_install() when a tty is
>> initialized, and released in ipoctal_cleanup() when the tty is finally
>> destroyed. The remove callback replaces direct kfree() with kref_put(),
>> ensuring the memory is only freed after all tty references have been
>> released.
>>
>> However, keeping the ipoctal struct alive via kref exposed a null
>> pointer dereference: __ipoctal_remove() calls tty_port_free_xmit_buf()
>> which frees and NULLs xmit_buf, but a userspace process may still hold
>> the tty fd and call write(), leading to a crash in
>> ipoctal_copy_write_buffer() when it dereferences xmit_buf.
>>
>> Fix this by checking for a NULL xmit_buf in ipoctal_write_tty() and
>> returning 0 if the buffer has been freed.
>>
>> 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 | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
>> index 1bbefc6de708..6bbbfb651dea 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,7 @@ struct ipoctal {
>> struct tty_driver *tty_drv;
>> u8 __iomem *mem8_space;
>> u8 __iomem *int_space;
>> + struct kref kref;
>> };
>>
>> static inline struct ipoctal *chan_to_ipoctal(struct ipoctal_channel *chan,
>> @@ -95,6 +99,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;
>> @@ -462,6 +467,9 @@ static ssize_t ipoctal_write_tty(struct tty_struct *tty, const u8 *buf,
>> struct ipoctal_channel *channel = tty->driver_data;
>> size_t char_copied;
>>
>> + if (!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 */
>> @@ -666,6 +674,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 +692,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 +708,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 +719,7 @@ static int ipoctal_probe(struct ipack_device *dev)
>> return 0;
>>
>> out_uninst:
>> - kfree(ipoctal);
>> + kref_put(&ipoctal->kref, ipoctal_release);
>> return res;
>> }
>>
>> @@ -725,7 +743,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
>>