From: Bryan O'Donoghue <bod@kernel.org>
To: Guangshuo Li <lgs201920130244@gmail.com>,
Vikash Garodia <vikash.garodia@oss.qualcomm.com>,
Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Stanimir Varbanov <stanimir.varbanov@linaro.org>,
Hans Verkuil <hans.verkuil@cisco.com>,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: venus: venc: avoid double free on video register failure
Date: Tue, 19 May 2026 11:09:56 +0100 [thread overview]
Message-ID: <8787ea87-aa75-4fb5-a729-cd2b54d2ff8a@kernel.org> (raw)
In-Reply-To: <20260519090819.1041314-1-lgs201920130244@gmail.com>
On 19/05/2026 10:08, Guangshuo Li wrote:
> venc_probe() allocates a video_device with video_device_alloc() and
> releases it from the err_vdev_release error path if
> video_register_device() fails.
>
> This can double free the video_device when __video_register_device()
> reaches device_register() and that call fails:
>
> video_register_device()
> -> __video_register_device()
> -> device_register() fails
> -> put_device(&vdev->dev)
> -> v4l2_device_release()
> -> vdev->release(vdev)
> -> video_device_release(vdev)
>
> venc_probe()
> -> err_vdev_release
> -> video_device_release(vdev)
>
> Use video_device_release_empty() while registering the device so that
> registration failure paths do not free vdev through vdev->release().
> venc_probe() then releases vdev exactly once from err_vdev_release.
> Restore video_device_release() after successful registration so the
> registered device keeps its normal lifetime handling.
>
> This issue was found by a static analysis tool I am developing.
>
> Fixes: aaaa93eda64b ("[media] media: venus: venc: add video encoder files")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/media/platform/qcom/venus/venc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index bf53267cb68d..9a5a025607fb 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -1579,7 +1579,7 @@ static int venc_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> strscpy(vdev->name, "qcom-venus-encoder", sizeof(vdev->name));
> - vdev->release = video_device_release;
> + vdev->release = video_device_release_empty;
> vdev->fops = &venc_fops;
> vdev->ioctl_ops = &venc_ioctl_ops;
> vdev->vfl_dir = VFL_DIR_M2M;
> @@ -1590,6 +1590,7 @@ static int venc_probe(struct platform_device *pdev)
> if (ret)
> goto err_vdev_release;
>
> + vdev->release = video_device_release;
> core->vdev_enc = vdev;
> core->dev_enc = dev;
>
> --
> 2.43.0
>
OK so this will get the same feedback as the Iris version which is
please fix the cleanup path.
If we look at drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c we can see
ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_VIDEO, -1);
if (ret) {
dev_err(dev, "failed to register video device\n");
goto err_vdev_register;
}
<snip>
err_vdev_register:
/* Only release if allocation succeeded but registration failed */
if (jpeg->dec_vdev)
video_device_release(jpeg->dec_vdev);
So for Venus and Iris
err_vdev_release:
if(vdev)
video_device_release(vdev);
i.e. only release the video device on the error path if the vdev pointer
is non-NULL.
---
bod
next prev parent reply other threads:[~2026-05-19 10:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <xMdPPQAJ2BbtNwnxmf1CN7FGbdhSJM7NIXkRCxzFvXv0g01tuvNPvAacsFJaDyBc3cIkIAEfi44ewZ3OGGAcDg==@protonmail.internalid>
2026-05-19 9:08 ` [PATCH] media: venus: venc: avoid double free on video register failure Guangshuo Li
2026-05-19 10:09 ` Bryan O'Donoghue [this message]
2026-05-19 12:51 ` Guangshuo Li
2026-05-19 13:20 ` Bryan O'Donoghue
2026-05-19 14:58 ` Guangshuo Li
2026-05-19 16:34 ` Bryan O'Donoghue
2026-05-20 2:31 ` Guangshuo Li
2026-05-20 6:10 ` Hans Verkuil
2026-05-20 8:19 ` Guangshuo Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8787ea87-aa75-4fb5-a729-cd2b54d2ff8a@kernel.org \
--to=bod@kernel.org \
--cc=dikshita.agarwal@oss.qualcomm.com \
--cc=hans.verkuil@cisco.com \
--cc=lgs201920130244@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=stanimir.varbanov@linaro.org \
--cc=vikash.garodia@oss.qualcomm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.