Re: [PATCH] media: s5p-mfc: avoid double free on video register failure
From: Marek Szyprowski <m.szyprowski@samsung.com>
Date: 2026-05-20 11:37:55
Also in:
linux-media, lkml
On 18.05.2026 15:09, Guangshuo Li wrote:
s5p_mfc_probe() allocates video_device instances for both the decoder
and encoder and releases them from the probe error paths if
video_register_device() fails.
This can double free a 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)
s5p_mfc_probe()
-> err_dec_reg or err_enc_reg
-> video_device_release(vdev)
Use video_device_release_empty() while registering the decoder and encoder
video devices so that registration failure paths do not free them through
vdev->release(). s5p_mfc_probe() then releases each video_device exactly
once from its error path. Restore video_device_release() after successful
registration so the registered devices keep their normal lifetime
handling.
This issue was found by a static analysis tool I am developing.
Fixes: d0ce898c39bf ("[media] s5p-mfc: Replaced commas with semicolons")
Signed-off-by: Guangshuo Li <redacted>Frankly speaking I don't like this dancing with video_device_release_empty() and video_device_release(). I would rather make video_device struct a part of device state and use common release function.
quoted hunk ↗ jump to hunk
--- drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c index 32eb402d439c..75abb0a8b7a9 100644 --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c@@ -1376,7 +1376,7 @@ static int s5p_mfc_probe(struct platform_device *pdev) } vfd->fops = &s5p_mfc_fops; vfd->ioctl_ops = get_dec_v4l2_ioctl_ops(); - vfd->release = video_device_release; + vfd->release = video_device_release_empty; vfd->lock = &dev->mfc_mutex; vfd->v4l2_dev = &dev->v4l2_dev; vfd->vfl_dir = VFL_DIR_M2M;@@ -1395,7 +1395,7 @@ static int s5p_mfc_probe(struct platform_device *pdev) } vfd->fops = &s5p_mfc_fops; vfd->ioctl_ops = get_enc_v4l2_ioctl_ops(); - vfd->release = video_device_release; + vfd->release = video_device_release_empty; vfd->lock = &dev->mfc_mutex; vfd->v4l2_dev = &dev->v4l2_dev; vfd->vfl_dir = VFL_DIR_M2M;@@ -1416,6 +1416,8 @@ static int s5p_mfc_probe(struct platform_device *pdev) v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); goto err_dec_reg; } + + dev->vfd_dec->release = video_device_release; v4l2_info(&dev->v4l2_dev, "decoder registered as /dev/video%d\n", dev->vfd_dec->num);@@ -1424,6 +1426,8 @@ static int s5p_mfc_probe(struct platform_device *pdev) v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); goto err_enc_reg; } + + dev->vfd_enc->release = video_device_release; v4l2_info(&dev->v4l2_dev, "encoder registered as /dev/video%d\n", dev->vfd_enc->num);
Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland