Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 12 Oct 2023 19:03:28 +0200 | Subject | Re: [PATCH] Subject: thermal: Fix potential race condition in suspend/resume |
| |
On Thu, Oct 12, 2023 at 5:39 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 16/09/2023 13:33, Bo Ye wrote: > > From: "yugang.wang" <yugang.wang@mediatek.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`. > > I'm not sure the tasks are already thawed during POST_RESTORE, so no > user can unload a driver and then reaching the race window.
Yes, they are.
> Is that an observed issue?
Good question, but the patch looks correct to me.
> > 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@mediatek.com> > > Signed-off-by: Bo Ye <bo.ye@mediatek.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_UNSPECIFIED); > > } > > + mutex_unlock(&thermal_list_lock); > > break; > > default: > > break; > > --
| |