linux-kernel - Re: [PATCH] thermal: Fix potential race condition in suspend/resume

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Oct 2023 01:19:59 +0000
From:   Bo Ye (叶波) <Bo.Ye@...iatek.com>
To:     "rui.zhang@...el.com" <rui.zhang@...el.com>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "amitk@...nel.org" <amitk@...nel.org>,
        "rafael@...nel.org" <rafael@...nel.org>
CC:     Yongdong Zhang (张永东) 
        <Yongdong.Zhang@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Browse Zhang (张磊) 
        <Browse.Zhang@...iatek.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        Yugang Wang (王玉刚) 
        <Yugang.Wang@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] thermal: Fix potential race condition in suspend/resume

Yes, it is observed issue.
Firstly, it needs to be clarified that this issue occurs in a real-
world environment. By analyzing the logs, we inferred that the issue
occurred just as the system was entering suspend mode, and the user was
switching the thermal policy (this action causes all thermal zones to
unregister/register). In addition, we conducted degradation tests and
also reproduced this issue. The specific method is to first switch the
thermal policy through a command, and then immediately put the system
into suspend state through another command. This method can also
reproduce the issue.

On Thu, 2023-10-12 at 07:35 +0000, Bo Ye (叶波) wrote:
> On Sat, 2023-09-16 at 19:33 +0800, Bo Ye wrote:
> 
> Correct mail title format: remove "Subject:" from mail title.
> 
> > From: "yugang.wang" <yugang.wang@...iatek.com>
> > 
> > Body:
> > This patch fixes a race condition during system resume. It occurs
> > if
> > the system is exiting a suspend state and a user is trying to
> > register/unregister a thermal zone concurrently. The root cause is
> > that both actions access the `thermal_tz_list`.
> > 
> > In detail:
> > 
> > 1. At PM_POST_SUSPEND during the resume, the system reads all
> > thermal
> >    zones in `thermal_tz_list`, then resets and updates their
> >    temperatures.
> > 2. When registering/unregistering a thermal zone, the
> >    `thermal_tz_list` gets manipulated.
> > 
> > These two actions might occur concurrently, causing a race
> > condition.
> > To solve this issue, we introduce a mutex lock to protect
> > `thermal_tz_list` from being modified while it's being read and
> > updated during the resume from suspend.
> > 
> > Kernel oops excerpt related to this fix:
> > 
> > [ 5201.869845] [T316822] pc: [0xffffffeb7d4876f0]
> > mutex_lock+0x34/0x170
> > [ 5201.869856] [T316822] lr: [0xffffffeb7ca98a84]
> > thermal_pm_notify+0xd4/0x26c
> > [... cut for brevity ...]
> > [ 5201.871061] [T316822]  suspend_prepare+0x150/0x470
> > [ 5201.871067] [T316822]  enter_state+0x84/0x6f4
> > [ 5201.871076] [T316822]  state_store+0x15c/0x1e8
> > 
> > Change-Id: Ifdbdecba17093f91eab7e36ce04b46d311ca6568
> > Signed-off-by: yugang.wang <yugang.wang@...iatek.com>
> > Signed-off-by: Bo Ye <bo.ye@...iatek.com>
> > ---
> >  drivers/thermal/thermal_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/thermal/thermal_core.c
> > b/drivers/thermal/thermal_core.c
> > index 8717a3343512..a7a18ed57b6d 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -1529,12 +1529,14 @@ static int thermal_pm_notify(struct
> > notifier_block *nb,
> >  	case PM_POST_HIBERNATION:
> >  	case PM_POST_RESTORE:
> >  	case PM_POST_SUSPEND:
> > +		mutex_lock(&thermal_list_lock);
> >  		atomic_set(&in_suspend, 0);
> >  		list_for_each_entry(tz, &thermal_tz_list, node) {
> >  			thermal_zone_device_init(tz);
> >  			thermal_zone_device_update(tz,
> >  						   THERMAL_EVENT_UNSPEC
> > IFIED);
> >  		}
> > +		mutex_unlock(&thermal_list_lock);
> >  		break;
> >  	default:
> >  		break;