Re: [PATCH V2] commdi: pcl726: Prevent invalid irq number

[Ian Abbott]

On 07/07/2025 12:43, Edward Adam Davis wrote:
> The reproducer passed in an irq number(0x80008000) that was too large,
> which triggered the oob.
>
> Added an interrupt number check to prevent users from passing in an irq
> number that was too large.
>
> If `it->options[1]` is 31, then `1 << it->options[1]` is still invalid
> because it shifts a 1-bit into the sign bit (which is UB in C).
> Possible solutions include reducing the upper bound on the
> `it->options[1]` value to 30 or lower, or using `1U << it->options[1]`.
>
> The old code would just not attempt to request the IRQ if the
> `options[1]` value were invalid.  And it would still configure the
> device without interrupts even if the call to `request_irq` returned an
> error.  So it would be better to combine this test with the test below.
>
> Fixes: fff46207245c ("staging: comedi: pcl726: enable the interrupt support code")
> Cc: <stable@xxxxxxxxxxxxxxx> # 5.13+
> Reported-by: syzbot+5cd373521edd68bebcb3@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=5cd373521edd68bebcb3
> Tested-by: syzbot+5cd373521edd68bebcb3@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> ---
> V1 -> V2: combine test with old test
>
>   drivers/comedi/drivers/pcl726.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/comedi/drivers/pcl726.c b/drivers/comedi/drivers/pcl726.c
> index 0430630e6ebb..b542896fa0e4 100644
> --- a/drivers/comedi/drivers/pcl726.c
> +++ b/drivers/comedi/drivers/pcl726.c
> @@ -328,7 +328,8 @@ static int pcl726_attach(struct comedi_device *dev,
>   	 * Hook up the external trigger source interrupt only if the
>   	 * user config option is valid and the board supports interrupts.
>   	 */
> -	if (it->options[1] && (board->irq_mask & (1 << it->options[1]))) {
> +	if (it->options[1] > 0 && it->options[1] < 16 &&
> +	    (board->irq_mask & (1U << it->options[1]))) {
>   		ret = request_irq(it->options[1], pcl726_interrupt, 0,
>   				  dev->board_name, dev);
>   		if (ret == 0) {

Looks good apart from the subject line that still has the "comedi" tag 
misspelled.  Please could you fix that?

You can also add my Reviewed-by: line:

Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

Please also Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> because 
I do not have commit access to any pulled git repos.

Thanks!

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