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 for Android: free password hash cracker in your pocket

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

Message-id: <20131004115612.1017e0f7@amdc308.digital.local>
Date:	Fri, 04 Oct 2013 11:56:12 +0200
From:	Lukasz Majewski <l.majewski@...sung.com>
To:	Eduardo Valentin <eduardo.valentin@...com>
Cc:	Zhang Rui <rui.zhang@...el.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

Hi Eduardo,

Thanks for reply.

> 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.

I have just preserved the old behaviour for -rcX fix.

> 
> 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.

It looks like we don't need this check (we will not register the
thermal zone if therm dev is not correct).

> 
> 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?

I think that it is up to you to decide how we proceed. 

Shall this patch go into v3.12-rcX since it is "obvious" (and documents
what and when was broken) or would you require to prepare "new" patch
with removal of this check?

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



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/