[PATCH] media: s5p-jpeg: avoid double free on video register failure

Guangshuo Li posted 1 patch 6 days, 16 hours ago
drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] media: s5p-jpeg: avoid double free on video register failure
Posted by Guangshuo Li 6 days, 16 hours ago
s5p_jpeg_probe() allocates encoder and decoder video_devices with
video_device_alloc() and releases them 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)

  s5p_jpeg_probe()
    -> video_device_release(jpeg->vfd_encoder)

The same issue exists for jpeg->vfd_decoder.

Use video_device_release_empty() while registering the devices so that
registration failure paths do not free them through vdev->release().
s5p_jpeg_probe() then releases the failed video_device exactly once.
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: 7a1d4e7c064c ("[media] s5p-jpeg: Eliminate double kfree()")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
index fdc9d7e0be66..046ad555c5fc 100644
--- a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
@@ -2915,7 +2915,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	jpeg->vfd_encoder->fops		= &s5p_jpeg_fops;
 	jpeg->vfd_encoder->ioctl_ops	= &s5p_jpeg_ioctl_ops;
 	jpeg->vfd_encoder->minor	= -1;
-	jpeg->vfd_encoder->release	= video_device_release;
+	jpeg->vfd_encoder->release	= video_device_release_empty;
 	jpeg->vfd_encoder->lock		= &jpeg->lock;
 	jpeg->vfd_encoder->v4l2_dev	= &jpeg->v4l2_dev;
 	jpeg->vfd_encoder->vfl_dir	= VFL_DIR_M2M;
@@ -2927,6 +2927,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 		video_device_release(jpeg->vfd_encoder);
 		goto m2m_init_rollback;
 	}
+	jpeg->vfd_encoder->release = video_device_release;
 
 	video_set_drvdata(jpeg->vfd_encoder, jpeg);
 	v4l2_info(&jpeg->v4l2_dev,
@@ -2945,7 +2946,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	jpeg->vfd_decoder->fops		= &s5p_jpeg_fops;
 	jpeg->vfd_decoder->ioctl_ops	= &s5p_jpeg_ioctl_ops;
 	jpeg->vfd_decoder->minor	= -1;
-	jpeg->vfd_decoder->release	= video_device_release;
+	jpeg->vfd_decoder->release	= video_device_release_empty;
 	jpeg->vfd_decoder->lock		= &jpeg->lock;
 	jpeg->vfd_decoder->v4l2_dev	= &jpeg->v4l2_dev;
 	jpeg->vfd_decoder->vfl_dir	= VFL_DIR_M2M;
@@ -2957,6 +2958,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 		video_device_release(jpeg->vfd_decoder);
 		goto enc_vdev_register_rollback;
 	}
+	jpeg->vfd_decoder->release = video_device_release;
 
 	video_set_drvdata(jpeg->vfd_decoder, jpeg);
 	v4l2_info(&jpeg->v4l2_dev,
-- 
2.43.0