From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f178.google.com (mail-qk1-f178.google.com [209.85.222.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B2D5536C9E4 for ; Tue, 30 Jun 2026 19:11:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782846682; cv=none; b=Up8fm8Mf3EV571gwhk6g09Cls3/r7cYG3DfjsCNW97hLr1KJmczgBY4/ayhJJsJwjV4KIw2DfSfKTM09pznxAgM8Ww+nk8Fsp8g7uxnSXSPLA6bwC/4shRtex+9PCcvqPAxn5Cla5mhPpCAU9NSVY4Yk0JUAZw7PWkNCH/l5xnk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782846682; c=relaxed/simple; bh=2BOXZZ5SpsTXt0QfQDjdqfgrW/imQErDY2wrshXxAuQ=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=oln+q1Q63vxjT9XfAAqLUvynit7ouzRH84F5MSScIXywi21e+BfSqYVwE1o9AjY0U+4hzsoerM2c4C4yfyb8mFN/NpSgz9Whad93rzE/UFWzo2XJtHUKO6LnOuEJhxNRMaGR/4ZqucBFogEioU9gr524nTJ2sHu8xV63QWJeQ/o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=qaFNFQrq; arc=none smtp.client-ip=209.85.222.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="qaFNFQrq" Received: by mail-qk1-f178.google.com with SMTP id af79cd13be357-92e65e18969so111093585a.1 for ; Tue, 30 Jun 2026 12:11:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782846680; x=1783451480; darn=vger.kernel.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=1V4+lgPOe0VQNOctf/MiFQy72WqPQWHV5J41gC+Uj0M=; b=qaFNFQrqrK1eu6ubUolGXw3tpTyom81qmx1cRRNJzsZ8LoxXusaxRc1GAahRVOf6cF 3uiY9PYWFQxdNsYOCu6egpDgHh+gh0pWamjtpQZGaHc9UYsLgZ3B3IZNBQMGGqLnTE9T vRqWYgbC7MP96pI0EzXgXjDXdQ2CxKhZGcdbAxhHPNdECeREfg3v87Z7NNwYtcKx1iaD 8e5QnyQgkQ7x9T40cc8kxLx3/nMl3WF8wfyEmlJ5WVpJd+1fj7JU1WuQtM63dqBi09AF 3eeKQgtz8SxRbtEb8odrWqNxGwS/S12XPnzHtNAG45Zz6wywdPP7gTECSF2Jxk4yqhmV bO9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782846680; x=1783451480; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=1V4+lgPOe0VQNOctf/MiFQy72WqPQWHV5J41gC+Uj0M=; b=QODkXd71jXce4GvQLJkukGf4CCeskFS/A7pf31vLIKEHk5vv33IYvBAajMmriskpSc rKBVCjrXqvJroTtMxnYn5Yhx5OAyqKLn3o6e+UfTRf7RUcWbglwjVL3x9mY6qtGLlqc2 3edg66GLb29QYXq6rUBwYLRaDaoIe8tpbKyPW9/ElQQGi45o3XdYQOtVYERHKRcHolyQ ZT/4j8yfCYkLYZYPfzeqAme4MD2uqz6DV5bHskDPEgdWiHfSA1iMISUD536yLnQu37fL ab5hYB7FaHbmTbiXC17iVZyu+Jsu6NuKb6KAoLpoBqJOeig46TgFNuNkXF7TyGHCghF5 XcMA== X-Gm-Message-State: AOJu0YxNSJWjhj/B8oa5A9te4E9f7gb7K6lPlfo+/N1U4wp586glMQZS T0OLIG5g3gQGOPZ2jQoaZFX46+j3/PY+pSRcoOZmNp0FScYQipvd+6t263UwXaX2MLU= X-Gm-Gg: AfdE7ck2Szy9W9agLEwFR/9zm4FgAU40czehnxutYkjjL6btBkV3IhLyjuMDK5hC3g6 IHDgZU9Al1pI+EeefcXqGr4+vKbf1xvKhfDmQbgnbFpVjA/Vff/sI+r79ey4aQqpWA3PAnuEvxt nDAvs/uiJX9x3cKhrpRxMCutLOErFlpfRPHc6A+Wdt9cdTA4bDY3EoA4mc5+RPRAM/OHQEpyqBX wub5SSzp61b8Op3Oea37tLlo6s+ie3RuezwWA7ZOcuG+FzVK8tGXEPh54joLWEcyIzGYEP85Eqc QO4iG68iwg2wH1d2NBmpGxdkyOYTN3hYRSXZLNYvCeqC0CQBRBXSEp4/etHQ2zsjKuOrCIF+OIx Nh4ardU8u/uFLhzmB+o+tGGt8dXdX5JEjO/R4Szdu+eUH5WFt43EghRYAgB5utul2luKyyU8+xj cnyAX/bIW9NTj/izQ4/ixTmHIFAwNm+Jag52G7uUhM84zzDEDu2Jev X-Received: by 2002:a05:620a:4549:b0:915:5216:e5bf with SMTP id af79cd13be357-92e6976f660mr420588385a.22.1782846679508; Tue, 30 Jun 2026 12:11:19 -0700 (PDT) Received: from smtpclient.apple ([2601:985:4601:5df0:f548:f1d7:86e:e6b3]) by smtp.gmail.com with ESMTPSA id af79cd13be357-92e6218fa3dsm309454385a.17.2026.06.30.12.11.18 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 Jun 2026 12:11:19 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\)) Subject: Re: [PATCH 1/2] ipack: ipoctal: fix UAF, null-ptr-deref, and use-after-free in cleanup on remove From: Shuangpeng In-Reply-To: Date: Tue, 30 Jun 2026 15:10:48 -0400 Cc: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <2C294A3B-5F5F-4329-A9E4-E165F69AB62F@gmail.com> References: <28C40F77-46D3-4ABC-BAD2-38EB9A606E71@gmail.com> To: Pei Xiao X-Mailer: Apple Mail (2.3864.600.51.1.1) Hi Pei, I tried this patch on my side and no bug is triggered. Thanks for your fix! Best, Shuangpeng > On Jun 30, 2026, at 03:40, Pei Xiao wrote: >=20 > hi shuangpeng, >=20 > If this version have no bugs. I will send it to Greg for review. > Thanks! >=20 > =E5=9C=A8 2026/6/26 10:31, Pei Xiao =E5=86=99=E9=81=93: >> Three issues arise when the device is removed while a tty session is >> still active: >>=20 >> 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 =E2=80=94 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(). >>=20 >> 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(). >>=20 >> 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. >>=20 >> 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. >>=20 >> Reported-by: Shuangpeng Bai >> Closes: = https://lore.kernel.org/lkml/178144969601.60470.1257088106279546587@gmail.= com/ >> Fixes: 05e5027efc9c ("Staging: ipack: move out of staging") >> Signed-off-by: Pei Xiao >> --- >> drivers/ipack/devices/ipoctal.c | 56 = ++++++++++++++++++++++++++++++--- >> 1 file changed, 52 insertions(+), 4 deletions(-) >>=20 >> 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 >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -25,6 +26,8 @@ >>=20 >> static const struct tty_operations ipoctal_fops; >>=20 >> +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; >> }; >>=20 >> 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; >>=20 >> channel =3D dev_get_drvdata(tty->dev); >> + ipoctal =3D chan_to_ipoctal(channel, tty->index); >> + >> + >> + if (ipoctal->removed) >> + return -ENODEV; >>=20 >> /* >> * 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; >>=20 >> + kref_get(&ipoctal->kref); >> tty->driver_data =3D channel; >>=20 >> 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 =3D tty->driver_data; >> + struct ipoctal *ipoctal =3D chan_to_ipoctal(channel, tty->index); >> size_t char_copied; >>=20 >> + >> + if (ipoctal->removed || !channel->tty_port.xmit_buf) >> + return 0; >> + >> char_copied =3D ipoctal_copy_write_buffer(channel, buf, count); >>=20 >> /* 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 =3D 0; >> unsigned char csr =3D 0; >> struct ipoctal_channel *channel =3D tty->driver_data; >> + struct ipoctal *ipoctal =3D chan_to_ipoctal(channel, tty->index); >> speed_t baud; >>=20 >> + >> + if (ipoctal->removed) >> + return; >> + >> cflag =3D tty->termios.c_cflag; >>=20 >> /* 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 =3D tty->driver_data; >> + struct ipoctal *ipoctal; >>=20 >> if (channel =3D=3D NULL) >> return; >>=20 >> + ipoctal =3D chan_to_ipoctal(channel, tty->index); >> + >> + if (ipoctal->removed) >> + return; >> + >> spin_lock_irqsave(&channel->lock, flags); >> channel->nb_bytes =3D 0; >> channel->pointer_read =3D 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 =3D tty->driver_data; >> + struct ipoctal *ipoctal; >>=20 >> if (channel =3D=3D NULL) >> return; >>=20 >> + ipoctal =3D 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 =3D tty->driver_data; >> struct ipoctal *ipoctal =3D chan_to_ipoctal(channel, tty->index); >>=20 >> - /* 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); >> } >>=20 >> static const struct tty_operations ipoctal_fops =3D { >> @@ -683,6 +719,13 @@ static const struct tty_operations ipoctal_fops = =3D { >> .cleanup =3D ipoctal_cleanup, >> }; >>=20 >> +static void ipoctal_release(struct kref *kref) >> +{ >> + struct ipoctal *ipoctal =3D 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 =3D=3D NULL) >> return -ENOMEM; >>=20 >> + kref_init(&ipoctal->kref); >> + >> ipoctal->dev =3D dev; >> + ipoctal->carrier_owner =3D dev->bus->owner; >> res =3D 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; >>=20 >> out_uninst: >> - kfree(ipoctal); >> + kref_put(&ipoctal->kref, ipoctal_release); >> return res; >> } >>=20 >> @@ -709,6 +755,8 @@ static void __ipoctal_remove(struct ipoctal = *ipoctal) >> { >> int i; >>=20 >> + ipoctal->removed =3D true; >> + >> ipoctal->dev->bus->ops->free_irq(ipoctal->dev); >>=20 >> for (i =3D 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); >> } >>=20 >> static void ipoctal_remove(struct ipack_device *idev) >=20