Re: [PATCH 3/6] media: pwc: Return queued buffers on start_streaming() failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete Buffer Cleanup on Error in `start_streaming()` causing a buffer leak
--

commit 39f02233e64ac5cb48aca4fd3aaf5db7ee99320b
Author: Valery Borovsky <vebohr@gmail.com>
Subject: media: pwc: Return queued buffers on start_streaming() failure

This commit addresses leaked buffers during start_streaming() failures
by ensuring queued buffers are returned to videobuf2 before returning an
error on the early exit paths.

Cc: stable@vger.kernel.org

> diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
> --- a/drivers/media/usb/pwc/pwc-if.c
> +++ b/drivers/media/usb/pwc/pwc-if.c
> @@ -710,11 +710,15 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>  	struct pwc_device *pdev = vb2_get_drv_priv(vq);
>  	int r;
>  
> -	if (!pdev->udev)
> +	if (!pdev->udev) {
> +		pwc_cleanup_queued_bufs(pdev, VB2_BUF_STATE_QUEUED);

Does this cleanup catch buffers that might have been moved to pdev->fill_buf?

While this isn't a new regression introduced by this specific patch, it
looks like a buffer leak could still happen if pwc_isoc_init() fails later
in this function.

During pwc_isoc_init(), usb_submit_urb() is called with GFP_KERNEL and
can sleep. If a previously submitted URB completes successfully before the
loop finishes, its completion handler pwc_isoc_handler() can execute and
move a buffer from pdev->queued_bufs into pdev->fill_buf.

If a subsequent usb_submit_urb() fails, pwc_isoc_init() aborts and kills
the URBs. The error path in start_streaming() then executes
pwc_cleanup_queued_bufs(). Since pdev->fill_buf is no longer in the list,
it would be ignored by the cleanup routine and leaked, triggering the
same WARN_ON(owned_by_drv_count) trap.

Should pdev->fill_buf be explicitly checked and returned to videobuf2 on
failure, similar to how it is handled in stop_streaming()?

>  		return -ENODEV;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778518085.git.vebohr@gmail.com?part=3