Re: [PATCH] comedi: dt2811: fix shift-out-of-bounds in dt2811_attach()

[Ian Abbott]

On 2026-09-23 07:08, Rohinthan P wrote:
> In dt2811_attach(), the driver checks whether the user-supplied IRQ in
> it->options[1] is valid for async command support using:
>
>      if (it->options[1] <= 7 && (BIT(it->options[1]) & 0xac))
>
> Because it->options[1] is a signed integer, passing a negative value
> (such as -251) satisfies it->options[1] <= 7 and causes the BIT()
> macro to evaluate a negative shift exponent (1UL << -251), resulting in
> undefined behavior and triggering a UBSAN shift-out-of-bounds warning:
>
>      UBSAN: shift-out-of-bounds in drivers/comedi/drivers/dt2811.c
>      shift exponent -251 is negative
>      CPU: 0 UID: 0 PID: 5925 Comm: syz.0.17 Not tainted
>      Call Trace:
>       dt2811_attach.cold+0x19/0x1e drivers/comedi/drivers/dt2811.c:570
>       comedi_device_attach+0x40e/0x6b0 drivers/comedi/drivers.c:1101
>       do_devconfig_ioctl+0x1b3/0x6d0 drivers/comedi/comedi_fops.c:930
>
> Only IRQs 2, 3, 5, and 7 are valid on the DT2811. Add a check that
> it->options[1] >= 2 before evaluating BIT(it->options[1]) to avoid
> negative shift values.
>
> Fixes: 8ffdff6a8cfb ("staging: comedi: move out of staging directory")
> Reported-by: syzbot+a4cedab706d0a8a3f988@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=a4cedab706d0a8a3f988
> Signed-off-by: Rohinthan P <rokinthanp03@xxxxxxxxx>
> ---
>   drivers/comedi/drivers/dt2811.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/comedi/drivers/dt2811.c b/drivers/comedi/drivers/dt2811.c
> index bcc4b5e..813b1aa 100644
> --- a/drivers/comedi/drivers/dt2811.c
> +++ b/drivers/comedi/drivers/dt2811.c
> @@ -564,7 +564,8 @@ static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>   	dt2811_reset(dev);
>   
>   	/* IRQ's 2,3,5,7 are valid for async command support */
> -	if (it->options[1] <= 7  && (BIT(it->options[1]) & 0xac)) {
> +	if (it->options[1] >= 2 && it->options[1] <= 7 &&
> +	    (BIT(it->options[1]) & 0xac)) {
>   		ret = request_irq(it->options[1], dt2811_interrupt, 0,
>   				  dev->board_name, dev);
>   		if (ret == 0)

Thanks for the fix.  Your patch looks fine, but there was another patch 
posted recently that fixes the same bug in a similar way (it used >= 0 
instead of >= 2) :

https://lore.kernel.org/lkml/20260922091939.GKvofurpBYesWLI8gPcW8CiW4eRuvtPwH7-hn1IVL8o@z/

--
-=( Ian Abbott <abbotti@xxxxxxxxx> || 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 )=-