[PATCH] media: uvcvideo: Fix bug in error path of uvc_alloc_urb_buffers

Ricardo Ribalda posted 1 patch 2 weeks, 3 days ago
drivers/media/usb/uvc/uvc_video.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
[PATCH] media: uvcvideo: Fix bug in error path of uvc_alloc_urb_buffers
Posted by Ricardo Ribalda 2 weeks, 3 days ago
Recent cleanup introduced a bug in the error path of
uvc_alloc_urb_buffers(). If there is not enough memory for the
allocation the following error will be triggered:

[  739.196672] UBSAN: shift-out-of-bounds in mm/page_alloc.c:1403:22
[  739.196710] shift exponent 52 is too large for 32-bit type 'int'

Resulting in:
[  740.464422] BUG: unable to handle page fault for address: fffffac1c0800000

The reason for the bug is that usb_free_noncoherent is called with an
invalid size (0) instead of the actual size of the urb.

This patch takes care of that.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Closes: https://lore.kernel.org/linux-media/abycbXzYupZpGkvR@hyeyoo/T/#t
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Fixes: c824345288d1 ("media: uvcvideo: Pass allocation size directly to uvc_alloc_urb_buffer")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 40c76c051da2..f6c8e3223796 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1751,7 +1751,8 @@ static void uvc_video_complete(struct urb *urb)
 /*
  * Free transfer buffers.
  */
-static void uvc_free_urb_buffers(struct uvc_streaming *stream)
+static void uvc_free_urb_buffers(struct uvc_streaming *stream,
+				 unsigned int size)
 {
 	struct usb_device *udev = stream->dev->udev;
 	struct uvc_urb *uvc_urb;
@@ -1760,7 +1761,7 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
 		if (!uvc_urb->buffer)
 			continue;
 
-		usb_free_noncoherent(udev, stream->urb_size, uvc_urb->buffer,
+		usb_free_noncoherent(udev, size, uvc_urb->buffer,
 				     uvc_stream_dir(stream), uvc_urb->sgt);
 		uvc_urb->buffer = NULL;
 		uvc_urb->sgt = NULL;
@@ -1820,7 +1821,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
 
 			if (!uvc_alloc_urb_buffer(stream, uvc_urb, urb_size,
 						  gfp_flags)) {
-				uvc_free_urb_buffers(stream);
+				uvc_free_urb_buffers(stream, urb_size);
 				break;
 			}
 
@@ -1868,7 +1869,7 @@ static void uvc_video_stop_transfer(struct uvc_streaming *stream,
 	}
 
 	if (free_buffers)
-		uvc_free_urb_buffers(stream);
+		uvc_free_urb_buffers(stream, stream->urb_size);
 }
 
 /*

---
base-commit: a93a51f42ac354425a252210183c4151d991f75d
change-id: 20260320-uvc-urb-free-error-e73b1db16e21

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

Re: [PATCH] media: uvcvideo: Fix bug in error path of uvc_alloc_urb_buffers
Posted by Laurent Pinchart 1 week, 6 days ago
On Fri, Mar 20, 2026 at 07:49:10AM +0000, Ricardo Ribalda wrote:
> Recent cleanup introduced a bug in the error path of
> uvc_alloc_urb_buffers(). If there is not enough memory for the
> allocation the following error will be triggered:
> 
> [  739.196672] UBSAN: shift-out-of-bounds in mm/page_alloc.c:1403:22
> [  739.196710] shift exponent 52 is too large for 32-bit type 'int'
> 
> Resulting in:
> [  740.464422] BUG: unable to handle page fault for address: fffffac1c0800000
> 
> The reason for the bug is that usb_free_noncoherent is called with an
> invalid size (0) instead of the actual size of the urb.
> 
> This patch takes care of that.
> 
> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Closes: https://lore.kernel.org/linux-media/abycbXzYupZpGkvR@hyeyoo/T/#t
> Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Fixes: c824345288d1 ("media: uvcvideo: Pass allocation size directly to uvc_alloc_urb_buffer")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This is a v7.0 fix. Hans, Mauro, could you please pick it up ?

> ---
>  drivers/media/usb/uvc/uvc_video.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 40c76c051da2..f6c8e3223796 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1751,7 +1751,8 @@ static void uvc_video_complete(struct urb *urb)
>  /*
>   * Free transfer buffers.
>   */
> -static void uvc_free_urb_buffers(struct uvc_streaming *stream)
> +static void uvc_free_urb_buffers(struct uvc_streaming *stream,
> +				 unsigned int size)
>  {
>  	struct usb_device *udev = stream->dev->udev;
>  	struct uvc_urb *uvc_urb;
> @@ -1760,7 +1761,7 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
>  		if (!uvc_urb->buffer)
>  			continue;
>  
> -		usb_free_noncoherent(udev, stream->urb_size, uvc_urb->buffer,
> +		usb_free_noncoherent(udev, size, uvc_urb->buffer,
>  				     uvc_stream_dir(stream), uvc_urb->sgt);
>  		uvc_urb->buffer = NULL;
>  		uvc_urb->sgt = NULL;
> @@ -1820,7 +1821,7 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
>  
>  			if (!uvc_alloc_urb_buffer(stream, uvc_urb, urb_size,
>  						  gfp_flags)) {
> -				uvc_free_urb_buffers(stream);
> +				uvc_free_urb_buffers(stream, urb_size);
>  				break;
>  			}
>  
> @@ -1868,7 +1869,7 @@ static void uvc_video_stop_transfer(struct uvc_streaming *stream,
>  	}
>  
>  	if (free_buffers)
> -		uvc_free_urb_buffers(stream);
> +		uvc_free_urb_buffers(stream, stream->urb_size);
>  }
>  
>  /*
> 
> ---
> base-commit: a93a51f42ac354425a252210183c4151d991f75d
> change-id: 20260320-uvc-urb-free-error-e73b1db16e21

-- 
Regards,

Laurent Pinchart