linux-kernel - [PATCH] comedi: pcl818: fix null-ptr-deref in pcl818_ai

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20251020161208.4151618-1-n.zhandarovich@fintech.ru>
Date: Mon, 20 Oct 2025 19:12:06 +0300
From: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
To: Ian Abbott <abbotti@....co.uk>, H Hartley Sweeten
	<hsweeten@...ionengravers.com>
CC: Nikita Zhandarovich <n.zhandarovich@...tech.ru>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>, Hillf Danton
	<hdanton@...a.com>, <lvc-project@...uxtesting.org>
Subject: [PATCH] comedi: pcl818: fix null-ptr-deref in pcl818_ai_cancel()

Syzbot identified an issue [1] in pcl818_ai_cancel(), which stems from
the fact that in case of early device detach via pcl818_detach(),
subdevice dev->read_subdev may not have initialized its pointer to
&struct comedi_async as intended. Thus, any such dereferencing of
&s->async->cmd will lead to general protection fault and kernel crash.

Mitigate this problem by checking in advance whether async struct is
properly present.

Original idea for this patch belongs to Hillf Danton
<hdanton@...a.com>.

Note: as suggested by Ian in [2], there may be a different solution to
improving the way ISRs are set up (and possibly, this flaw as well).
However, until that idea (based on reference counters and completion
structs) comes to fruition, it makes sense to fix this now, if only
to stop kernel crashing so often during syzkaller runs.

[1] Syzbot crash:
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
CPU: 1 UID: 0 PID: 6050 Comm: syz.0.18 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/18/2025
RIP: 0010:pcl818_ai_cancel+0x69/0x3f0 drivers/comedi/drivers/pcl818.c:762
...
Call Trace:
 <TASK>
 pcl818_detach+0x66/0xd0 drivers/comedi/drivers/pcl818.c:1115
 comedi_device_detach_locked+0x178/0x750 drivers/comedi/drivers.c:207
 do_devconfig_ioctl drivers/comedi/comedi_fops.c:848 [inline]
 comedi_unlocked_ioctl+0xcde/0x1020 drivers/comedi/comedi_fops.c:2178
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:597 [inline]
...

[2] Ian's suggestion on how to fix interrupt handlers in comedi drivers:
Link: https://lore.kernel.org/all/9c92913c-c04b-4784-9cdc-5d75b10d2ed9@mev.co.uk/

Reported-by: syzbot+fce5d9d5bd067d6fbe9b@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fce5d9d5bd067d6fbe9b
Suggested-by: Hillf Danton <hdanton@...a.com>
Fixes: 00aba6e7b565 ("staging: comedi: pcl818: remove 'neverending_ai' from private data")
Cc: stable@...r.kernel.org
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
---
P.S. Tested with syzkaller reproducers as well as by doing a trial
syzkaller run w/wo the patch.

 drivers/comedi/drivers/pcl818.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/comedi/drivers/pcl818.c b/drivers/comedi/drivers/pcl818.c
index 4127adcfb229..3c6cfb212492 100644
--- a/drivers/comedi/drivers/pcl818.c
+++ b/drivers/comedi/drivers/pcl818.c
@@ -759,12 +759,15 @@ static int pcl818_ai_cancel(struct comedi_device *dev,
 {
 	struct pcl818_private *devpriv = dev->private;
 	struct comedi_isadma *dma = devpriv->dma;
-	struct comedi_cmd *cmd = &s->async->cmd;
+	struct comedi_cmd *cmd;

 	if (!devpriv->ai_cmd_running)
 		return 0;

 	if (dma) {
+		if (!s || !s->async)
+			return 0;
+		cmd = &s->async->cmd;
 		if (cmd->stop_src == TRIG_NONE ||
 		    (cmd->stop_src == TRIG_COUNT &&
 		     s->async->scans_done < cmd->stop_arg)) {