[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f0f48940-213b-494e-88f5-0275874fe044@mev.co.uk>
Date: Wed, 22 Oct 2025 11:38:53 +0100
From: Ian Abbott <abbotti@....co.uk>
To: Nikita Zhandarovich <n.zhandarovich@...tech.ru>,
H Hartley Sweeten <hsweeten@...ionengravers.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org,
syzbot+7811bb68a317954a0347@...kaller.appspotmail.com
Subject: Re: [PATCH] comedi: multiq3: sanitize config options in
multiq3_attach()
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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7811bb68a317954a0347
> Fixes: 77e01cdbad51 ("Staging: comedi: add multiq3 driver")
> Cc: stable@...r.kernel.org
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@...tech.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@....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 )=-
Powered by blists - more mailing lists