linux-kernel - Re: [PATCH v2] thermal: Fix a NULL pointer dereference

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: <1e0371e7-5c15-a552-870a-a501ec11a8f9@quicinc.com>
Date:   Thu, 21 Oct 2021 11:29:20 -0700
From:   Subbaraman Narayanamurthy <quic_subbaram@...cinc.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Amit Kucheria <amitk@...nel.org>
CC:     <linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        David Collins <quic_collinsd@...cinc.com>,
        Manaf Meethalavalappu Pallikunhi <manafm@...eaurora.org>,
        Ram Chandrasekar <rkumbako@...eaurora.org>,
        <stable@...r.kernel.org>
Subject: Re: [PATCH v2] thermal: Fix a NULL pointer dereference

On 10/19/21 12:35 AM, Daniel Lezcano wrote:
> On 19/10/2021 03:21, Subbaraman Narayanamurthy wrote:
>> On 10/8/21 12:50 PM, Subbaraman Narayanamurthy wrote:
>>> On 10/6/21 4:08 AM, Daniel Lezcano wrote:
> [ ... ]
>
>>> /sys/devices/virtual/thermal/thermal_zone87 # echo 120000 > trip_point_0_temp  
>>> [  184.290964][  T211] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020
>>> [  184.300896][  T211] Mem abort info:                                         
>>> [  184.304486][  T211]   ESR = 0x96000006                                      
>>> [  184.308348][  T211]   EC = 0x25: DABT (current EL), IL = 32 bits            
>>> [  184.314531][  T211]   SET = 0, FnV = 0                                      
>>> [  184.318384][  T211]   EA = 0, S1PTW = 0                                     
>>> [  184.322323][  T211] Data abort info:                                        
>>> [  184.325993][  T211]   ISV = 0, ISS = 0x00000006                             
>>> [  184.330655][  T211]   CM = 0, WnR = 0                                       
>>> [  184.334425][  T211] user pgtable: 4k pages, 39-bit VAs, pgdp=000000081a7a2000
>>> [  184.341750][  T211] [0000000000000020] pgd=000000081a7a7003, p4d=000000081a7a7003, pud=000000081a7a7003, pmd=0000000000000000
>>> [  184.353359][  T211] Internal error: Oops: 96000006 [#1] PREEMPT SMP         
>>> [  184.359797][  T211] Dumping ftrace buffer:                                  
>>> [  184.364001][  T211]    (ftrace buffer empty)
>>>
>>> Hope this helps.
>> Hi Daniel,
>> Have you got a chance to look at this?
> Hi Subbaraman,
>
> Actually, I think the root problem is the thermal zone is showing up
> while there is no sensor associated with it. You can read the
> temperature and get a kernel warning also.
>
> That's what should be fixed IMO.
>

Hi Daniel,

If I understand your statement correctly, are you saying that a thermal_zone device should be created only after a thermal sensor driver supplying it probes?

>From what I can see,

thermal_init()
    --> of_parse_thermal_zones()
            --> thermal_zone_device_register()
                    --> thermal_zone_create_device_groups()
                            <followed by>
                        device_register() with thermal_zone%d


This happens way before a thermal sensor driver probes. So, creating a thermal_zone device only after its thermal sensor probes would require more changes to the framework.

Also, I see a similar NULL check exists in the framework code already.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/thermal/thermal_of.c?h=v5.15-rc6#n103

So, extending the logic for other callsites (below) makes sense to me.

- of_thermal_get_temp()
- of_thermal_set_emul_temp()
- of_thermal_get_trend()
- of_thermal_set_trip_temp()

Thanks,
Subbaraman