UBSAN: shift-out-of-bounds in set_flicker
Subsystems (2)
PATCH v2 1/1 UBSAN: shift-out-of-bounds in set_flicker
From: Rajeshwar R Shinde <[email protected]>
Syzkaller reported the following issue:
UBSAN: shift-out-of-bounds in drivers/media/usb/gspca/cpia1.c:1031:27
shift exponent 245 is too large for 32-bit type 'int'
shift-out-of-bounds error was triggered when variable 'sd->params.exposure.gain' is greater than the number of bits of int. When the variable 'currentexp' is left shifted beyond 31 bits then the error is produced. Therefore added the conditional expression to verify valid range.
Tested via syzbot.
---v1->v2
changed the patch
changed commit message and tested with checkpatch
--- drivers/media/usb/gspca/cpia1.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/media/usb/gspca/cpia1.c +2 -0
@@ -1028,6 +1028,8 @@ static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply) sd->params.exposure.expMode =2 ; sd->exposure_status =EXPOSURE_NORMAL ; } +if (sd->params.exposure.gain >31 ) +return -1 ; currentexp = currentexp << sd->params.exposure.gain; sd->params.exposure.gain =0 ;/* round down current exposure to nearest value */
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 13 lines · On Thu, Aug 24, 2023 at 12:36:30PM +0530, [email protected] wrote:
From: Rajeshwar R Shinde <[email protected]>
Syzkaller reported the following issue:
UBSAN: shift-out-of-bounds in drivers/media/usb/gspca/cpia1.c:1031:27shift exponent 245 is too large for 32-bit type 'int'
shift-out-of-bounds error was triggered when variable '
sd->params.exposure.gain' is greater than the number of bits of int. When the variable 'currentexp' is left shifted beyond 31 bits then the error is produced. Therefore added the conditional expression to verify valid range.
Quoted 8 lines
Tested via syzbot.
Reported-by [email protected]Signed-off-by Rajeshwar R Shinde <[email protected]>
Please do not have blank lines beween these tags.
You also have trailing whitespace in your changelog text :(
Quoted 14 lines
---
v1->v2
changed the patch
changed commit message and tested with checkpatch--- drivers/media/usb/gspca/cpia1.c | 2 ++ 1 file changed, 2 insertions(+)drivers/media/usb/gspca/cpia1.c +0
-0
drivers/media/usb/gspca/cpia1.c static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply) · line 1032
Quoted 5 lines
drivers/media/usb/gspca/cpia1.c +2
-0@@ -1028,6 +1028,8 @@ static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply) sd->params.exposure.expMode =2 ; sd->exposure_status =EXPOSURE_NORMAL ; } +if (sd->params.exposure.gain >31 ) +return -1 ;
Do not make up error codes, please return a valid one and not a random negative number. Unless -1 is a valid value for this function?
thanks,
greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 19 lines · Greg KH
Quoted 18 lines · On Thu, Aug 24, 2023 at 12:36:30PM +0530, [email protected] wrote:
From: Rajeshwar R Shinde <[email protected]>
Syzkaller reported the following issue:
UBSAN: shift-out-of-bounds in drivers/media/usb/gspca/cpia1.c:1031:27shift exponent 245 is too large for 32-bit type 'int'
shift-out-of-bounds error was triggered when variable '
sd->params.exposure.gain' is greater than the number of bits of int. When the variable 'currentexp' is left shifted beyond 31 bits then the error is produced. Therefore added the conditional expression to verify valid range.Tested via syzbot.
Reported-by [email protected]
Quoted 7 lines
Quoted 3 lines
Signed-off-by Rajeshwar R Shinde <[email protected]>Please do not have blank lines beween these tags.
You also have trailing whitespace in your changelog text :(
And please mention the driver name in your subject! E.g. something like:
"media: gspca/cpia1: UBSAN: shift-out-of-bounds in set_flicker"
That way I can actually know that this is a patch for a driver that I maintain.
Regards,
Hans
Quoted 24 lines
Quoted 15 lines
---
v1->v2
changed the patch
changed commit message and tested with checkpatch--- drivers/media/usb/gspca/cpia1.c | 2 ++ 1 file changed, 2 insertions(+)drivers/media/usb/gspca/cpia1.c +0
-0drivers/media/usb/gspca/cpia1.c static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply) · line 1032
Quoted 5 lines
drivers/media/usb/gspca/cpia1.c +2
-0@@ -1028,6 +1028,8 @@ static int set_flicker(struct gspca_dev *gspca_dev, int on, int apply) sd->params.exposure.expMode =2 ; sd->exposure_status =EXPOSURE_NORMAL ; } +if (sd->params.exposure.gain >31 ) +return -1 ;Do not make up error codes, please return a valid one and not a random negative number. Unless -1 is a valid value for this function?
Quoted 3 lines
thanks,
greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees