diff mbox series

[1/2] media: cedrus: Fix missing cleanup in error path

Message ID 20260401191441.1217646-1-andrej.skvortzov@gmail.com (mailing list archive)
State New
Headers
Series [1/2] media: cedrus: Fix missing cleanup in error path |

Commit Message

Andrey Skvortsov April 1, 2026, 7:14 p.m. UTC 
From: Samuel Holland <samuel@sholland.org>

From: Samuel Holland <samuel@sholland.org>

According to the documentation struct v4l2_fh has to be cleaned up with
v4l2_fh_exit() before being freed. [1]

1. https://docs.kernel.org/driver-api/media/v4l2-fh.html

Signed-off-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Dan Carpenter April 2, 2026, 1:09 p.m. UTC | #1 
On Wed, Apr 01, 2026 at 10:14:40PM +0300, Andrey Skvortsov wrote:
> From: Samuel Holland <samuel@sholland.org>
> 
> From: Samuel Holland <samuel@sholland.org>
> 
> According to the documentation struct v4l2_fh has to be cleaned up with
> v4l2_fh_exit() before being freed. [1]
> 
> 1. https://docs.kernel.org/driver-api/media/v4l2-fh.html
> 

I wish the commit message would say what the use visible effect of the
bug is.  I looked at it and I don't think this patch hurts but I also
didn't necessarily see a that the original code had a user visible bug.

I read the documentation but it wasn't as unambiguous as I'd prefer.

But I'm not a subsystem expert.

regards,
dan carpenter
Andrey Skvortsov April 6, 2026, 9:23 p.m. UTC | #2 
Hi,

On 26-04-02 16:09, Dan Carpenter wrote:
> On Wed, Apr 01, 2026 at 10:14:40PM +0300, Andrey Skvortsov wrote:
> > From: Samuel Holland <samuel@sholland.org>
> > 
> > From: Samuel Holland <samuel@sholland.org>
> > 
> > According to the documentation struct v4l2_fh has to be cleaned up with
> > v4l2_fh_exit() before being freed. [1]
> > 
> > 1. https://docs.kernel.org/driver-api/media/v4l2-fh.html
> > 
> 
> I wish the commit message would say what the use visible effect of the
> bug is.  I looked at it and I don't think this patch hurts but I also
> didn't necessarily see a that the original code had a user visible bug.
> 
> I read the documentation but it wasn't as unambiguous as I'd prefer.
> 

Thank you for the review. Currently there is no visible
bug. v4l2_fh_exit() in this case only destroys mutex.
But it may change in the future, when v4l2_fh_init/v4l2_fh_exit will
be changed. I think the change maybe useful in this regard.

I'll describe this in the commit message in v2 and resend it separately from the
patch 2, that fixes actual problem. So this change may be skipped, if
maintainers think it's not worth to apply.
diff mbox series

Patch

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 6600245dff0e2..1d2130f35fffc 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -391,6 +391,7 @@  static int cedrus_open(struct file *file)
 err_m2m_release:
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 err_free:
+	v4l2_fh_exit(&ctx->fh);
 	kfree(ctx);
 	mutex_unlock(&dev->dev_mutex);