[PATCH] rv: Fix missing mutex unlock in rv_register_monitor()

[PATCH] rv: Fix missing mutex unlock in rv_register_monitor()

[Zhen Ni]

If create_monitor_dir() fails, the function returns directly without
releasing rv_interface_lock. This leaves the mutex locked and causes
subsequent monitor registration attempts to deadlock.

Fix it by making the error path jump to out_unlock, ensuring that the
mutex is always released before returning.

Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct rv_monitor")
Signed-off-by: Zhen Ni <zhen.ni@easystack.cn>
---
 kernel/trace/rv/rv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index 1482e91c39f4..e35565dd2dc5 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -805,7 +805,7 @@ int rv_register_monitor(struct rv_monitor *monitor, struct rv_monitor *parent)
 
 	retval = create_monitor_dir(monitor, parent);
 	if (retval)
-		return retval;
+		goto out_unlock;
 
 	/* keep children close to the parent for easier visualisation */
 	if (parent)
-- 
2.20.1

Re: [PATCH] rv: Fix missing mutex unlock in rv_register_monitor()

[Gabriele Monaco]

On Wed, 2025-09-03 at 14:51 +0800, Zhen Ni wrote:
> If create_monitor_dir() fails, the function returns directly without
> releasing rv_interface_lock. This leaves the mutex locked and causes
> subsequent monitor registration attempts to deadlock.
> 
> Fix it by making the error path jump to out_unlock, ensuring that the
> mutex is always released before returning.
> 
> Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct
> rv_monitor")
> Signed-off-by: Zhen Ni <zhen.ni@easystack.cn>

Good catch, thank you!

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

I would also add a:

Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct rv_monitor")

(Adding Nam to the loop, as author of that patch)

Thanks,
Gabriele

> ---
>  kernel/trace/rv/rv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index 1482e91c39f4..e35565dd2dc5 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -805,7 +805,7 @@ int rv_register_monitor(struct rv_monitor
> *monitor, struct rv_monitor *parent)
>  
>  	retval = create_monitor_dir(monitor, parent);
>  	if (retval)
> -		return retval;
> +		goto out_unlock;
>  
>  	/* keep children close to the parent for easier
> visualisation */
>  	if (parent)

Re: [PATCH] rv: Fix missing mutex unlock in rv_register_monitor()

[Nam Cao]

On Thu, Sep 04, 2025 at 09:07:38AM +0200, Gabriele Monaco wrote:
> On Wed, 2025-09-03 at 14:51 +0800, Zhen Ni wrote:
> > If create_monitor_dir() fails, the function returns directly without
> > releasing rv_interface_lock. This leaves the mutex locked and causes
> > subsequent monitor registration attempts to deadlock.
> > 
> > Fix it by making the error path jump to out_unlock, ensuring that the
> > mutex is always released before returning.
> > 
> > Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct
> > rv_monitor")
> > Signed-off-by: Zhen Ni <zhen.ni@easystack.cn>
> 
> Good catch, thank you!
> 
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> 
> I would also add a:
> 
> Fixes: 24cbfe18d55a ("rv: Merge struct rv_monitor_def into struct rv_monitor")

Am I hallucinating, or the fix tag is already there?

> (Adding Nam to the loop, as author of that patch)

Thanks for CC!

Reviewed-by: Nam Cao <namcao@linutronix.de>

I have been debating whether we should convert rv to use lock guard.
Problem like this one is a good advocate.

Nam

Re: [PATCH] rv: Fix missing mutex unlock in rv_register_monitor()

[Gabriele Monaco]

On Thu, 2025-09-04 at 09:19 +0200, Nam Cao wrote:
> Am I hallucinating, or the fix tag is already there?

I should stop doing 3 things at the same time, my bad :')

> 
> > (Adding Nam to the loop, as author of that patch)
> 
> Thanks for CC!
> 
> Reviewed-by: Nam Cao <namcao@linutronix.de>
> 
> I have been debating whether we should convert rv to use lock guard.
> Problem like this one is a good advocate.

I'm totally in favour, next cleanup series let's say.

Thanks,
Gabriele