Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: only update registered thermal zones

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Chen-Yu Tsai <wenst@chromium.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Amit Kucheria <amitk@kernel.org>, Zhang Rui <rui.zhang@intel.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	Balsam CHIHI <bchihi@baylibre.com>,
	linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: only update registered thermal zones
Date: Sat, 1 Apr 2023 22:34:14 +0200	[thread overview]
Message-ID: <9ab8d176-378c-9720-3794-c710e2bd6272@linaro.org> (raw)
In-Reply-To: <20230328031037.1361048-1-wenst@chromium.org>

On 28/03/2023 05:10, Chen-Yu Tsai wrote:
> It's possible for some sensors or thermal zones to not be registered,
> either because they are unused or not fully declared in the device tree.
> Nevertheless the driver enables interrupts for all sensors. If an
> interrupt happens for an not-registered sensor, the driver would end up
> updating a non-existent thermal zone, which leads to a NULL pointer
> dereference.
> 
> Change it so that only registered thermal zones get updated.

Why not change the interrupt initialization ?

> Fixes: f5f633b18234 ("thermal/drivers/mediatek: Add the Low Voltage Thermal Sensor driver")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>   drivers/thermal/mediatek/lvts_thermal.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index d87d3847c7d0..bf59174e18d3 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -415,9 +415,14 @@ static irqreturn_t lvts_ctrl_irq_handler(struct lvts_ctrl *lvts_ctrl)
>   		if (!(value & masks[i]))
>   			continue;
>   
> +		iret = IRQ_HANDLED;
> +
> +		/* sensor might not exist (bogus interrupt) or not be registered */
> +		if (!lvts_ctrl->sensors[i].tz)
> +			continue;
> +
>   		thermal_zone_device_update(lvts_ctrl->sensors[i].tz,
>   					   THERMAL_TRIP_VIOLATED);
> -		iret = IRQ_HANDLED;
>   	}
>   
>   	/*

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog