Skip to contentSkip to conversation

power: validate wakeup source before activating it.

PATCH Superseded Jin Qian 2 participants 2 messages May 5, 2015

Subsystems (4)
Driver core Suspend to RAM Hibernation Power management core

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jin Qian

author
#0 lore raw

PATCH v2 1/3 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 function.

Signed-off-by Jin Qian <[email protected]>
---
 drivers/base/power/wakeup.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/base/power/wakeup.c +19 -0
index 7726200..5d866b9 100644
@@ -14,6 +14,7 @@
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
+#include <linux/types.h>
 #include <trace/events/power.h>
 
 #include "power.h"

@@ -351,6 +352,19 @@ int device_set_wakeup_enable(struct device *dev, bool enable)
 }
 EXPORT_SYMBOL_GPL(device_set_wakeup_enable);
 
+/**
+ * validate_wakeup_source - validate the given wakeup source.
+ * @ws: Wakeup source to be validated.
+ */
+static bool validate_wakeup_source(struct wakeup_source *ws)
+{
+	/*
+	 * Use timer function to check if the given source is initialized
+	 * by wakeup_source_add.
+	 */
+	return ws->timer.function == pm_wakeup_timer_fn;
+}
+
 /*
  * 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,11 @@ static void wakeup_source_activate(struct wakeup_source *ws)
 {
 	unsigned int cec;
 
+	if (WARN_ON(!validate_wakeup_source(ws))) {
+		pr_err("unregistered wakeup source: %s\n", ws->name);
+		return;
+	}
+
 	/*
 	 * active wakeup source should bring the system
 	 * out of PM_SUSPEND_FREEZE state
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#1 lore raw
Quoted 14 lines · Jin Qian

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

Signed-off-by Jin Qian <[email protected]>
---
 drivers/base/power/wakeup.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/base/power/wakeup.c +0 -0
index 7726200..5d866b9 100644

drivers/base/power/wakeup.c line 17

Quoted 4 lines
drivers/base/power/wakeup.c +1 -0
@@ -14,6 +14,7 @@
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
+#include <linux/types.h>

What's that for?

drivers/base/power/wakeup.c int device_set_wakeup_enable(struct device *dev, bool enable) · line 359

Quoted 12 lines
drivers/base/power/wakeup.c +5 -0
 #include <trace/events/power.h>
 
 #include "power.h"

@@ -351,6 +352,19 @@ int device_set_wakeup_enable(struct device *dev, bool enable)
 }
 EXPORT_SYMBOL_GPL(device_set_wakeup_enable);
 
+/**
+ * validate_wakeup_source - validate the given wakeup source.
+ * @ws: Wakeup source to be validated.
+ */
+static bool validate_wakeup_source(struct wakeup_source *ws)

I would call that wakeup_source_not_registered() and do the reverse check. The extra logical negation below wouldn't be necessary then.

drivers/base/power/wakeup.c int device_set_wakeup_enable(struct device *dev, bool enable) · line 365

Quoted 6 lines
drivers/base/power/wakeup.c +6 -0
+{
+	/*
+	 * Use timer function to check if the given source is initialized
+	 * by wakeup_source_add.
+	 */
+	return ws->timer.function == pm_wakeup_timer_fn;

You can also verify if timer.data is equal to ws.

drivers/base/power/wakeup.c static void wakeup_source_activate(struct wakeup_source *ws) · line 408

Quoted 10 lines
drivers/base/power/wakeup.c +3 -0
+}
+
 /*
  * 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,11 @@ static void wakeup_source_activate(struct wakeup_source *ws)
 {
 	unsigned int cec;
 
+	if (WARN_ON(!validate_wakeup_source(ws))) {

WARN_ONCE() would be better, so you can make it print the message and you won't need the extra statement.

Quoted 8 lines
drivers/base/power/wakeup.c +4 -0
+		pr_err("unregistered wakeup source: %s\n", ws->name);
+		return;
+	}
+
 	/*
 	 * active wakeup source should bring the system
 	 * out of PM_SUSPEND_FREEZE state
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.