comedi: dt2811: fix shift-out-of-bounds in dt2811_attach()

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

Posted by Rohinthan P 1 day, 6 hours ago

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a4cedab706d0a8a3f988
Signed-off-by: Rohinthan P <rokinthanp03@gmail.com>
---
 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)
-- 
2.53.0

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

Posted by Ian Abbott 1 day, 3 hours ago

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a4cedab706d0a8a3f988
> Signed-off-by: Rohinthan P <rokinthanp03@gmail.com>
> ---
>   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@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 )=-