power: validate wakeup source before activating it.
Subsystems (4)
PATCH 1/1 power: validate wakeup source before activating it.
A rogue wakeup source not registered in wakeup_sources list is not visible from wakeup_sources_stats_show. Check if the wakeup source is registered properly by looking at the timer struct.
--- drivers/base/power/wakeup.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
drivers/base/power/wakeup.c +18 -0
@@ -351,6 +351,20 @@ int device_set_wakeup_enable(struct device *dev, bool enable) }EXPORT_SYMBOL_GPL (device_set_wakeup_enable); +/** +* wakeup_source_not_registered - validate the given wakeup source. +* @ws: Wakeup source to be validated. +*/ +static bool wakeup_source_not_registered (struct wakeup_source *ws) +{ +/* +* Use timer struct to check if the given source is initialized +* by wakeup_source_add. +*/ +return ws->timer.function != pm_wakeup_timer_fn || + ws->timer.data != (unsigned long )ws; +} +/* * The functions below use the observation that each wakeup event starts a * period in which the system should not be suspended. The moment this period
@@ -391,6 +405,10 @@ static void wakeup_source_activate(struct wakeup_source *ws) {unsigned int cec; +if (WARN_ONCE (wakeup_source_not_registered (ws), +"unregistered wakeup source\n" )) +return ; +/* * active wakeup source should bring the system * out of PM_SUSPEND_FREEZE state
Signature · 1 line
2.2.0.rc0.207.ga3a616c
Rafael J. Wysocki
AppliedQuoted 35 lines · Jin Qian
A rogue wakeup source not registered in
wakeup_sourceslist is not visible fromwakeup_sources_stats_show. Check if the wakeup source is registered properly by looking at the timer struct.Signed-off-by Jin Qian <[email protected]>--- drivers/base/power/wakeup.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/base/power/wakeup.c +14
-0@@ -351,6 +351,20 @@ int device_set_wakeup_enable(struct device *dev, bool enable) }EXPORT_SYMBOL_GPL (device_set_wakeup_enable); +/** +* wakeup_source_not_registered - validate the given wakeup source. +* @ws: Wakeup source to be validated. +*/ +static bool wakeup_source_not_registered (struct wakeup_source *ws) +{ +/* +* Use timer struct to check if the given source is initialized +* by wakeup_source_add. +*/ +return ws->timer.function != pm_wakeup_timer_fn || + ws->timer.data != (unsigned long )ws; +} +/* * The functions below use the observation that each wakeup event starts a * period in which the system should not be suspended. The moment this period
drivers/base/power/wakeup.c static void wakeup_source_activate(struct wakeup_source *ws) · line 414
Quoted 10 lines
drivers/base/power/wakeup.c +4
-0@@ -391,6 +405,10 @@ static void wakeup_source_activate(struct wakeup_source *ws) {unsigned int cec; +if (WARN_ONCE (wakeup_source_not_registered (ws), +"unregistered wakeup source\n" )) +return ; +/* * active wakeup source should bring the system * out of PM_SUSPEND_FREEZE state
Queued up for 4.2, thanks!
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Hi Rafael,
The latest version is in [PATCHv3 1/3] power: validate wakeup source before activating it. I changed WARN_ONCE back to WARN since if multiple drivers activating uninitialized wakeup_sources, only the first driver will by flagged. We lost alert for other drivers. The warning should really happen during driver development. Hope this is ok.
Thanks,
jin
Quoted 53 lines · Rafael J. Wysocki
Quoted 35 lines · Jin Qian
A rogue wakeup source not registered in
wakeup_sourceslist is not visible fromwakeup_sources_stats_show. Check if the wakeup source is registered properly by looking at the timer struct.Signed-off-by Jin Qian <[email protected]>--- drivers/base/power/wakeup.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)drivers/base/power/wakeup.c +14
-0@@ -351,6 +351,20 @@ int device_set_wakeup_enable(struct device *dev, bool enable) }EXPORT_SYMBOL_GPL (device_set_wakeup_enable); +/** +* wakeup_source_not_registered - validate the given wakeup source. +* @ws: Wakeup source to be validated. +*/ +static bool wakeup_source_not_registered (struct wakeup_source *ws) +{ +/* +* Use timer struct to check if the given source is initialized +* by wakeup_source_add. +*/ +return ws->timer.function != pm_wakeup_timer_fn || + ws->timer.data != (unsigned long )ws; +} +/* * The functions below use the observation that each wakeup event starts a * period in which the system should not be suspended. The moment this period drivers/base/power/wakeup.c static void wakeup_source_activate(struct wakeup_source *ws) · line 414
Quoted 10 lines
drivers/base/power/wakeup.c +4
-0@@ -391,6 +405,10 @@ static void wakeup_source_activate(struct wakeup_source *ws) {unsigned int cec; +if (WARN_ONCE (wakeup_source_not_registered (ws), +"unregistered wakeup source\n" )) +return ; +/* * active wakeup source should bring the system * out of PM_SUSPEND_FREEZE state Queued up for 4.2, thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
Quoted 6 lines · Jin Qian
Hi Rafael,
The latest version is in [PATCHv3 1/3] power: validate wakeup source before activating it. I changed
WARN_ONCEback to WARN since if multiple drivers activating uninitializedwakeup_sources, only the first driver will by flagged.
Which is OK.
If you're a user, you can't fix the problem by yourself and spamming the logs with continued warnings doesn't help here.
If you're a developer, you'll fix the first driver and then see the warning from the second one.
Quoted 2 lines
We lost alert for other drivers. The warning should really happen during driver development. Hope this is ok.
First, I've applied your patch already. Second, I really think that WARN_ON_ONCE() is better here.
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.