[PATCH] comedi: multiq3: sanitize config options in multiq3_attach()

Nikita Zhandarovich posted 1 patch 3 months, 2 weeks ago
There is a newer version of this series
drivers/comedi/drivers/multiq3.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH] comedi: multiq3: sanitize config options in multiq3_attach()
Posted by Nikita Zhandarovich 3 months, 2 weeks ago
Syzbot identified an issue [1] in multiq3_attach() that induces a
task timeout due to open() or COMEDI_DEVCONFIG ioctl operations,
specifically, in the case of multiq3 driver.

This problem arose when syzkaller managed to craft weird configuration
options used to specify the number of channels in encoder subdevice.
If a particularly great number is passed to s->n_chan in
multiq3_attach() via it->options[2], then multiple calls to
multiq3_encoder_reset() at the end of driver-specific attach() method
will be running for minutes, thus blocking tasks and affected devices
as well.

While this issue is most likely not too dangerous for real-life
devices, it still makes sense to sanitize configuration inputs. Enable
a semi-arbitrary limit on the number of encoder chips to stop this
behaviour from manifesting.

[1] Syzbot crash:
INFO: task syz.2.19:6067 blocked for more than 143 seconds.
...
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5254 [inline]
 __schedule+0x17c4/0x4d60 kernel/sched/core.c:6862
 __schedule_loop kernel/sched/core.c:6944 [inline]
 schedule+0x165/0x360 kernel/sched/core.c:6959
 schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7016
 __mutex_lock_common kernel/locking/mutex.c:676 [inline]
 __mutex_lock+0x7e6/0x1350 kernel/locking/mutex.c:760
 comedi_open+0xc0/0x590 drivers/comedi/comedi_fops.c:2868
 chrdev_open+0x4cc/0x5e0 fs/char_dev.c:414
 do_dentry_open+0x953/0x13f0 fs/open.c:965
 vfs_open+0x3b/0x340 fs/open.c:1097
...

Reported-by: syzbot+7811bb68a317954a0347@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7811bb68a317954a0347
Fixes: 77e01cdbad51 ("Staging: comedi: add multiq3 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
---
 drivers/comedi/drivers/multiq3.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/comedi/drivers/multiq3.c b/drivers/comedi/drivers/multiq3.c
index 07ff5383da99..0248321e3bfa 100644
--- a/drivers/comedi/drivers/multiq3.c
+++ b/drivers/comedi/drivers/multiq3.c
@@ -67,6 +67,11 @@
 #define MULTIQ3_TRSFRCNTR_OL		0x10	/* xfer CNTR to OL (x and y) */
 #define MULTIQ3_EFLAG_RESET		0x06	/* reset E bit of flag reg */
 
+/*
+ * Semi-arbitrary limit on the number of optional encoder chips
+ */
+#define MULTIQ3_MAX_ENC_CHIPS		16
+
 static void multiq3_set_ctrl(struct comedi_device *dev, unsigned int bits)
 {
 	/*
@@ -312,6 +317,10 @@ static int multiq3_attach(struct comedi_device *dev,
 	s->insn_read	= multiq3_encoder_insn_read;
 	s->insn_config	= multiq3_encoder_insn_config;
 
+	/* sanity check for number of optional encoders */
+	if (s->n_chan > MULTIQ3_MAX_ENC_CHIPS)
+		s->n_chan = MULTIQ3_MAX_ENC_CHIPS;
+
 	for (i = 0; i < s->n_chan; i++)
 		multiq3_encoder_reset(dev, i);
Re: [PATCH] comedi: multiq3: sanitize config options in multiq3_attach()
Posted by Ian Abbott 3 months, 2 weeks ago
On 21/10/2025 15:59, Nikita Zhandarovich wrote:
> Syzbot identified an issue [1] in multiq3_attach() that induces a
> task timeout due to open() or COMEDI_DEVCONFIG ioctl operations,
> specifically, in the case of multiq3 driver.
> 
> This problem arose when syzkaller managed to craft weird configuration
> options used to specify the number of channels in encoder subdevice.
> If a particularly great number is passed to s->n_chan in
> multiq3_attach() via it->options[2], then multiple calls to
> multiq3_encoder_reset() at the end of driver-specific attach() method
> will be running for minutes, thus blocking tasks and affected devices
> as well.
> 
> While this issue is most likely not too dangerous for real-life
> devices, it still makes sense to sanitize configuration inputs. Enable
> a semi-arbitrary limit on the number of encoder chips to stop this
> behaviour from manifesting.
> 
> [1] Syzbot crash:
> INFO: task syz.2.19:6067 blocked for more than 143 seconds.
> ...
> Call Trace:
>   <TASK>
>   context_switch kernel/sched/core.c:5254 [inline]
>   __schedule+0x17c4/0x4d60 kernel/sched/core.c:6862
>   __schedule_loop kernel/sched/core.c:6944 [inline]
>   schedule+0x165/0x360 kernel/sched/core.c:6959
>   schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7016
>   __mutex_lock_common kernel/locking/mutex.c:676 [inline]
>   __mutex_lock+0x7e6/0x1350 kernel/locking/mutex.c:760
>   comedi_open+0xc0/0x590 drivers/comedi/comedi_fops.c:2868
>   chrdev_open+0x4cc/0x5e0 fs/char_dev.c:414
>   do_dentry_open+0x953/0x13f0 fs/open.c:965
>   vfs_open+0x3b/0x340 fs/open.c:1097
> ...
> 
> Reported-by: syzbot+7811bb68a317954a0347@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7811bb68a317954a0347
> Fixes: 77e01cdbad51 ("Staging: comedi: add multiq3 driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
> ---
>   drivers/comedi/drivers/multiq3.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/comedi/drivers/multiq3.c b/drivers/comedi/drivers/multiq3.c
> index 07ff5383da99..0248321e3bfa 100644
> --- a/drivers/comedi/drivers/multiq3.c
> +++ b/drivers/comedi/drivers/multiq3.c
> @@ -67,6 +67,11 @@
>   #define MULTIQ3_TRSFRCNTR_OL		0x10	/* xfer CNTR to OL (x and y) */
>   #define MULTIQ3_EFLAG_RESET		0x06	/* reset E bit of flag reg */
>   
> +/*
> + * Semi-arbitrary limit on the number of optional encoder chips
> + */
> +#define MULTIQ3_MAX_ENC_CHIPS		16
> +
>   static void multiq3_set_ctrl(struct comedi_device *dev, unsigned int bits)
>   {
>   	/*
> @@ -312,6 +317,10 @@ static int multiq3_attach(struct comedi_device *dev,
>   	s->insn_read	= multiq3_encoder_insn_read;
>   	s->insn_config	= multiq3_encoder_insn_config;
>   
> +	/* sanity check for number of optional encoders */
> +	if (s->n_chan > MULTIQ3_MAX_ENC_CHIPS)
> +		s->n_chan = MULTIQ3_MAX_ENC_CHIPS;
> +
>   	for (i = 0; i < s->n_chan; i++)
>   		multiq3_encoder_reset(dev, i);
>   

Nice, but the limit on number of optional encoder chip channels 
(s->n_chan) seems to be 8 (it->options[2] = 4 encoder chips with 2 
channels per chip).  See the `MULTIQ_CTRL_E_CHAN(x)` macro:

#define MULTIQ3_CTRL_E_CHAN(x)		(((x) & 0x7) << 3)

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-