linux-kernel - Re: [PATCH 1/6] thermal: exynos: fix: Return from exynos_report_trigger() when therm

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]

Date:	Thu, 3 Oct 2013 17:40:46 -0400
From:	Eduardo Valentin <eduardo.valentin@...com>
To:	Lukasz Majewski <l.majewski@...sung.com>
CC:	Zhang Rui <rui.zhang@...el.com>,
	Eduardo Valentin <eduardo.valentin@...com>,
	Amit Daniel Kachhap <amit.daniel@...sung.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux PM list <linux-pm@...r.kernel.org>,
	Jonghwa Lee <jonghwa3.lee@...sung.com>,
	Lukasz Majewski <l.majewski@...ess.pl>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Tomasz Figa <t.figa@...sung.com>,
	Myungjoo Ham <myungjoo.ham@...sung.com>,
	<devicetree@...r.kernel.org>, <linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH 1/6] thermal: exynos: fix: Return from exynos_report_trigger()
 when therm_dev is NULL

On 24-09-2013 04:08, Lukasz Majewski wrote:
> The commit 4de0bdaa9677d11406c9becb70c60887c957e1f0
> ("thermal: exynos: Add support for instance based register/unregister")
> broke check for presence of therm_dev at global thermal zone in
> exynos_report_trigger().
> 
> The resulting wrong test prevents thermal_zone_device_update() call, which
> calls handlers for situation when trip points are passed.
> Such behavior prevents thermal driver from proper reaction (when TMU interrupt
> is raised) in a situation when overheating is detected at TMU hardware.
> This patch fixes it.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@...sung.com>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> Reviewed-by: Tomasz Figa <t.figa@...sung.com>
> ---
>  drivers/thermal/samsung/exynos_thermal_common.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c b/drivers/thermal/samsung/exynos_thermal_common.c
> index f10a6ad..55a912a 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -310,7 +310,7 @@ void exynos_report_trigger(struct thermal_sensor_conf *conf)
>  	}
>  
>  	th_zone = conf->pzone_data;
> -	if (th_zone->therm_dev)
> +	if (!th_zone->therm_dev)

Fine with this fix, as it really looks obvious. But after reading the
code a bit, I am considering if we can remove the above check at all.

Reading the driver code piece at drivers/thermal/samsung/exynos_tmu.c,
by checking how exynos_register_thermal gets called, and how error is
handled, I assume we do not need to check for th_zone->therm_dev.

To me looks like the driver only allows th_zone's exist only with valid
therm_dev, isn't it? Except for the probe sequence, maybe, at run time
there is a time window that we have valid th_zone with invalid
therm_dev. However, reading the probe, still, the irq gets initialized
only in very end, so the work queue don't get queue till then at least.

So, my question before acking is, shall we remove this check altogether?


>  		return;
>  
>  	if (th_zone->bind == false) {
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


Download attachment "signature.asc" of type "application/pgp-signature" (296 bytes)