Re: [syzbot] [kernel?] UBSAN: shift-out-of-bounds in das16m1_attach

[Enju, Kohei]

On 2025/07/07, 19:19, "Ian Abbott" <abbotti@xxxxxxxxx <mailto:abbotti@xxxxxxxxx>> wrote:

>On 04/07/2025 19:10, Kohei Enju wrote:
>> On Fri, 04 Jul 2025 09:20:29 -0700, syzbot wrote:
>>
>>> [...]
>>
>> A quick grep found similar patterns that could have the same issue.
>> I think we should validate it->options[1] before shifting, right?
>>
>>    $ grep -nrIF "<< it->options[1]" ./drivers/comedi
>>    ./drivers/comedi/drivers/aio_iiro_16.c:180:     if ((1 << it->options[1]) & 0xdcfc) {
>>    ./drivers/comedi/drivers/das16m1.c:526:     (1 << it->options[1]) & 0xdcfc) {
>>    ./drivers/comedi/drivers/das6402.c:570: if ((1 << it->options[1]) & 0x8cec) {
>>    ./drivers/comedi/drivers/pcl726.c:331:  if (it->options[1] && (board->irq_mask & (1 << it->options[1]))) {
>>    ./drivers/comedi/drivers/pcl812.c:1152:         if ((1 << it->options[1]) & board->irq_bits) {
>>
>> #syz test
>>
>> diff --git a/drivers/comedi/drivers/das16m1.c b/drivers/comedi/drivers/das16m1.c
>> index b8ea737ad3d1..1b638f5b5a4f 100644
>> --- a/drivers/comedi/drivers/das16m1.c
>> +++ b/drivers/comedi/drivers/das16m1.c
>> @@ -522,7 +522,8 @@ static int das16m1_attach(struct comedi_device *dev,
>>       devpriv->extra_iobase = dev->iobase + DAS16M1_8255_IOBASE;
>>
>>       /* only irqs 2, 3, 4, 5, 6, 7, 10, 11, 12, 14, and 15 are valid */
>> -     if ((1 << it->options[1]) & 0xdcfc) {
>> +     if (it->options[1] >= 2 && it->options[1] <= 15 &&
>> +         (1 << it->options[1]) & 0xdcfc) {
>>               ret = request_irq(it->options[1], das16m1_interrupt, 0,
>>                                 dev->board_name, dev);
>>               if (ret == 0)
>
>Thanks.
>
>That looks fine.  If you plan to submit an official patch, please Cc it
>to Greg KH because I do not have commit access to any pulled git repos.
>
>The Fixes: line for this is as follows:
>
>Fixes: 729988507680 ("staging: comedi: das16m1: tidy up the irq support
>in das16m1_attach()"
>
>Can also add:
>
>Cc: <stable@xxxxxxxxxxxxxxx> # 5.13+
>
>(Comedi moved out of staging in 5.13, so a backport is required for
>earlier longterm series.)
>
>If you don't want to deal with it, let me know, and I'll send the patch
>and credit you with a 'Suggested-by' tag, or something more explicit
>with your permission.

Thank you for taking a look and suggesting appropriate tags.

Unfortunately I don't have the bandwidth to prepare an official patch at the
moment, so would you (or anyone else) be able to handle it? I would appreciate
it if you could add a `Suggested-by` tag.

(To be honest, I think someone who knows this driver better than me would
write a superior patch.)

>--
>-=( 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 )=-

Thanks, 
Kohei.