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] [day] [month] [year] [list]

Message-ID: <CANAwSgTeZ83oqatrsWQxT+4RYwEtEqma=R4XX_iGrP2N=phz9Q@mail.gmail.com>
Date: Wed, 12 Mar 2025 20:29:19 +0530
From: Anand Moon <linux.amoon@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: 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

Hi Krzysztof,

Thanks for your review comments.

On Tue, 11 Mar 2025 at 23:00, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> 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
Ok.
>
> > 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.
 Ok, I will check this next time before submitting the changes.
>
> > ---
> >  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.

I thought this should add a global guard to exynos_tmu_data using
mutex_lock and mutex_unlock.
I'll drop this if it turns out to be unnecessary.
>
> > +          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.
>
Ok, I will drop this include of cleanup.h.
>
> Best regards,
> Krzysztof

Thanks
-Anand