linux-kernel - Re: [PATCH 3/5] thermal: rockchip: fixes invalid temperature case

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: <20161123043642.GA28948@google.com>
Date:   Tue, 22 Nov 2016 20:36:43 -0800
From:   Brian Norris <briannorris@...omium.org>
To:     Caesar Wang <wxt@...k-chips.com>
Cc:     heiko@...ech.de, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, smbarber@...omium.org,
        edubezval@...il.com, linux-rockchip@...ts.infradead.org,
        rui.zhang@...el.com
Subject: Re: [PATCH 3/5] thermal: rockchip: fixes invalid temperature case

On Wed, Nov 23, 2016 at 11:03:33AM +0800, Caesar Wang wrote:
> 在 2016年11月23日 10:33, Brian Norris 写道:
> >IIUC, "too high" should not be interpreted as TSADCV2_DATA_MASK on
> >rk3288, should it? That corresponds to -40C, which means you'll be
> >triggering the alarm temperature at a very *low* temperature, not a very
> >high one, no?
> 
> The "too high" will correspond to -40C on rk3288, but shouldn't
> trigger the alarm temperature.
> 
> Due to the alarm or tshut function will handle it.
> 
> e.g.:
> static void rk_tsadcv2_alarm_temp(const struct chip_tsadc_table *table,
>                   int chn, void __iomem *regs, int temp)
> {
>     u32 alarm_value, int_en;
> 
>     /* Make sure the value is valid */
>     alarm_value = rk_tsadcv2_temp_to_code(table, temp);
>     if (alarm_value == table->data_mask)
>         return;

Ah, right. I keep forgetting about this odd error handling.

That's still the wrong error handling though; the right response is
never to avoid doing anything (and therefore returning "success" to the
thermal core). You need to either program a high (or low) trip value, or
else report an error (i.e., allow rk_tsadcv2_alarm_temp() to return an
error code back to the calling function). Otherwise, this:

echo -45000 > trip_0_temp

will succeed without error, and:

cat trip_0_temp
-45000

will return the cached temperature from of-thermal, even though the trip
point is programmed to something else entirely.

Brian