On 29/09/2024 00:27, Al Viro wrote:

On Fri, Sep 27, 2024 at 04:02:32PM +0200, Philipp Zabel wrote:

quoted

Use guard(mutex) to automatically unlock mutexes when going out of
scope. Simplify error paths by removing a goto and manual mutex
unlocking in multiple places.

And that, folks, is a live example of the reasons why guard() is an
attractive nuisance.  We really need a very loud warning on
cleanup.h stuff - otherwise such patches from well-meaning folks
will keep coming.

quoted

@@ -1041,29 +1036,27 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
 		}
 	}
 
-	mutex_lock(&reset_list_mutex);
+	guard(mutex)(&reset_list_mutex);
 	rcdev = __reset_find_rcdev(&args, gpio_fallback);
 	if (!rcdev) {
 		rstc = ERR_PTR(-EPROBE_DEFER);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
 		rstc = ERR_PTR(-EINVAL);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	rstc_id = rcdev->of_xlate(rcdev, &args);
 	if (rstc_id < 0) {
 		rstc = ERR_PTR(rstc_id);
-		goto out_unlock;
+		goto out_put;
 	}
 
 	/* reset_list_mutex also protects the rcdev's reset_control list */
 	rstc = __reset_control_get_internal(rcdev, rstc_id, shared, acquired);
 
-out_unlock:
-	mutex_unlock(&reset_list_mutex);
 out_put:
 	of_node_put(args.np);

Guess what happens if you take goto out_put prior to the entire thing,
in
                ret = __reset_add_reset_gpio_device(&args);
		if (ret) {
			rstc = ERR_PTR(ret);
			goto out_put;
		}
That patch adds implicit mutex_unlock() at the points where we leave
the scope.  Which extends to the end of function.  In other words, there is
one downstream of out_put, turning any goto out_put upstream of guard() into
a bug.

cleanup.h also mentions that one should do not mix cleanup with existing
goto, because of possibility of above issue.

But except careful review, this patch should have been simply compile
tested which would point to the issue above. Any guard/scope works must
be checked with clang W=1, which reports jumps over init.

Best regards,
Krzysztof