Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RESEND PATCH v2] thermal: Fix a NULL pointer dereference
To:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
Cc:     Amit Kucheria <amitk@kernel.org>, Zhang Rui <rui.zhang@intel.com>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        David Collins <quic_collinsd@quicinc.com>,
        Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com>,
        Stable <stable@vger.kernel.org>
References: <1636070227-15909-1-git-send-email-quic_subbaram@quicinc.com>
 <CAJZ5v0gONybD_pVCAq6ZJTMuStXtoF064u9qPYxco4y=b-JD9A@mail.gmail.com>
From:   Daniel Lezcano <daniel.lezcano@linaro.org>
Message-ID: <c7ede029-b75f-e57e-24f1-9633d5d47401@linaro.org>
Date:   Fri, 5 Nov 2021 17:19:42 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <CAJZ5v0gONybD_pVCAq6ZJTMuStXtoF064u9qPYxco4y=b-JD9A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 05/11/2021 16:14, Rafael J. Wysocki wrote:
> On Fri, Nov 5, 2021 at 12:57 AM Subbaraman Narayanamurthy
> <quic_subbaram@quicinc.com> wrote:
>>
>> of_parse_thermal_zones() parses the thermal-zones node and registers a
>> thermal_zone device for each subnode. However, if a thermal zone is
>> consuming a thermal sensor and that thermal sensor device hasn't probed
>> yet, an attempt to set trip_point_*_temp for that thermal zone device
>> can cause a NULL pointer dereference. Fix it.
>>
>>  console:/sys/class/thermal/thermal_zone87 # echo 120000 > trip_point_0_temp
>>  ...
>>  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020
>>  ...
>>  Call trace:
>>   of_thermal_set_trip_temp+0x40/0xc4
>>   trip_point_temp_store+0xc0/0x1dc
>>   dev_attr_store+0x38/0x88
>>   sysfs_kf_write+0x64/0xc0
>>   kernfs_fop_write_iter+0x108/0x1d0
>>   vfs_write+0x2f4/0x368
>>   ksys_write+0x7c/0xec
>>   __arm64_sys_write+0x20/0x30
>>   el0_svc_common.llvm.7279915941325364641+0xbc/0x1bc
>>   do_el0_svc+0x28/0xa0
>>   el0_svc+0x14/0x24
>>   el0_sync_handler+0x88/0xec
>>   el0_sync+0x1c0/0x200
>>
>> While at it, fix the possible NULL pointer dereference in other
>> functions as well: of_thermal_get_temp(), of_thermal_set_emul_temp(),
>> of_thermal_get_trend().
> 
> Can the subject be more specific, please?
> 
> The issue appears to be limited to the of_thermal_ family of
> functions, but the subject doesn't reflect that at all.
> 
>> Suggested-by: David Collins <quic_collinsd@quicinc.com>
>> Signed-off-by: Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>
> 
> Daniel, any concerns regarding the code changes below?

I've a concern about the root cause but I did not have time to
investigate how to fix it nicely.

thermal_of is responsible of introducing itself between the thermal core
code and the backend. So it defines the ops which in turn call the
sensor ops leading us to this problem.

So, without a better solution, this fix can be applied until we rethink
the thermal_of approach.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>


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