Re: [PATCH] media: uvcvideo: Fix buffer overflow in uvc_mapping_get_menu_value()

[Ricardo Ribalda]

Hi Dan

I believe that for all the uses of uvc_mapping_get_menu_value we are
already doing bound checks:

index >= BITS_PER_TYPE(mapping->menu_mask) in uvc_query_v4l2_menu()
value> fls(mapping->menu_mask) -1 in uvc_ctrl_clamp()
BIT(i) <= mapping->menu_mask in uvc_menu_to_v4l2_menu()

In any case, I think this patch is still worthwhile. It will help us
avoiding bugs in the future (have you found this with a new test for
smatch?)
and if we go that way it is probably a good idea to also "fix"
uvc_mapping_get_menu_name()


On Fri, 18 Sept 2026 at 14:20, Dan Carpenter <error27@xxxxxxxxx> wrote:
>
> The "idx" value is a user controlled u32 so we have to bounds check it
> before calling test_bit() to avoid reading beyond the end of the bitmap.
Maybe change the commit message as well to avoid stable cherry picking
it blindly (sorry seems like I am more picky than usual  :) )

>
> Fixes: 4e15c535659b ("media: uvcvideo: Support any size for mapping get/set")
I believe this should be:
Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
or no Fixes at all.



> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 3ca108b83f1d..f157ed99be4e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -538,7 +538,7 @@ static void uvc_mapping_set_s32(struct uvc_control_mapping *mapping,
>  static int uvc_mapping_get_menu_value(const struct uvc_control_mapping *mapping,
>                                       u32 idx)
>  {
> -       if (!test_bit(idx, &mapping->menu_mask))
> +       if (idx >= BITS_PER_LONG || !test_bit(idx, &mapping->menu_mask))
>                 return -EINVAL;

What about using BITS_PER_TYPE(mapping->menu_mask) to be consistent?

>
>         if (mapping->menu_mapping)
> --
> 2.53.0
>

with those changes:
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
-- 
Ricardo Ribalda