linux-kernel - Re: [PATCH v4 4/4] drivers/thermal/exymos: Use guard notation when acquiring mutex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <25fa661b-98e4-468b-bb4d-4a2c95f32b71@kernel.org>
Date: Tue, 11 Mar 2025 18:30:07 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Anand Moon <linux.amoon@...il.com>,
 Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>,
 Lukasz Luba <lukasz.luba@....com>, Alim Akhtar <alim.akhtar@...sung.com>,
 "open list:SAMSUNG THERMAL DRIVER" <linux-pm@...r.kernel.org>,
 "open list:SAMSUNG THERMAL DRIVER" <linux-samsung-soc@...r.kernel.org>,
 "moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES"
 <linux-arm-kernel@...ts.infradead.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 4/4] drivers/thermal/exymos: Use guard notation when
 acquiring mutex

On 10/03/2025 15:34, Anand Moon wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
> 

Subject: typo, exynos

> Signed-off-by: Anand Moon <linux.amoon@...il.com>
> ---
> v4: used DEFINE_GUARD macro to guard exynos_tmu_data structure.
>     However, incorporating guard(exynos_tmu_data)(data); results
>     in a recursive deadlock with the mutex during initialization, as this
>     data structure is common to all the code configurations of Exynos TMU
> v3: New patch

If you ever use cleanup or guards, you must build your code with recent
clang and W=1. Failure to do so means you ask reviewers manually to spot
issues not visible in the context, instead of using tools. It's a NAK
for me.

> ---
>  drivers/thermal/samsung/exynos_tmu.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index a71cde0a4b17e..85f88c5e0f11c 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/cleanup.h>
>  #include <linux/io.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> @@ -199,6 +200,9 @@ struct exynos_tmu_data {
>  	void (*tmu_clear_irqs)(struct exynos_tmu_data *data);
>  };
>  
> +DEFINE_GUARD(exynos_tmu_data, struct exynos_tmu_data *,

I do not understand why do you need custom guard.

> +	     mutex_lock(&_T->lock), mutex_unlock(&_T->lock))
> +
>  /*
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the calibration type.
> @@ -256,7 +260,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>  	unsigned int status;
>  	int ret = 0;
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);

Which you do not use... Please don't use cleanup.h if you do not know
it. It leads to bugs.

Best regards,
Krzysztof