Skip to contentSkip to conversation

PM / wakeirq: check that wake IRQ is valid before accepting it

PATCH In review Dmitry Torokhov 3 participants 4 messages Nov 12, 2015 – Nov 13, 2015

Subsystems (4)
Driver core Suspend to RAM Hibernation Power management core
lore
Share & tools
lore

Dmitry Torokhov

author
#0 lore raw

PATCH 1/1 PM / wakeirq: check that wake IRQ is valid before accepting it

Check that IRQ number passed to dev_pm_set_wake_irq and dev_pm_set_dedicated_wake_irq is valid (not negative) before accepting it.

Signed-off-by Dmitry Torokhov <[email protected]>

---

My recent change to i2c core introduced a code path that led to calling dev_pm_set_wake_irq(&client->dev, -ENOENT), which succeeded but obviously did the wrong thing. Checking the IRQ and bailing out early would have helped noticing this issue earlier.

 drivers/base/power/wakeirq.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/base/power/wakeirq.c +6 -0
index eb6e674..0d77cd6 100644
@@ -68,6 +68,9 @@ int dev_pm_set_wake_irq(struct device *dev, int irq)
 	struct wake_irq *wirq;
 	int err;
 
+	if (irq < 0)
+		return -EINVAL;
+
 	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
 	if (!wirq)
 		return -ENOMEM;

@@ -167,6 +170,9 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 	struct wake_irq *wirq;
 	int err;
 
+	if (irq < 0)
+		return -EINVAL;
+
 	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
 	if (!wirq)
 		return -ENOMEM;
Signature · 5 lines
2.6.0.rc2.230.g3dd15c0


-- 
Dmitry
#1 lore raw
Quoted 19 lines · Dmitry Torokhov

Check that IRQ number passed to dev_pm_set_wake_irq and dev_pm_set_dedicated_wake_irq is valid (not negative) before accepting it.

Signed-off-by Dmitry Torokhov <[email protected]>

---

My recent change to i2c core introduced a code path that led to calling dev_pm_set_wake_irq(&client->dev, -ENOENT), which succeeded but obviously did the wrong thing. Checking the IRQ and bailing out early would have helped noticing this issue earlier.

  drivers/base/power/wakeirq.c | 6 ++++++
  1 file changed, 6 insertions(+)
drivers/base/power/wakeirq.c +0 -0
index eb6e674..0d77cd6 100644

drivers/base/power/wakeirq.c int dev_pm_set_wake_irq(struct device *dev, int irq) · line 71

Quoted 4 lines
drivers/base/power/wakeirq.c +1 -0
@@ -68,6 +68,9 @@ int dev_pm_set_wake_irq(struct device *dev, int irq)
  	struct wake_irq *wirq;
  	int err;
 
+	if (irq < 0)

<= 0 ?

Quoted 16 lines
drivers/base/power/wakeirq.c +5 -0
+		return -EINVAL;
+
  	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
  	if (!wirq)
  		return -ENOMEM;

@@ -167,6 +170,9 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
  	struct wake_irq *wirq;
  	int err;
 
+	if (irq < 0)
+		return -EINVAL;
+
  	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
  	if (!wirq)
  		return -ENOMEM;
Signature · 2 lines
regards,
-grygorii
author
#2 lore raw
Quoted 23 lines · Grygorii Strashko
Quoted 22 lines · Dmitry Torokhov

Check that IRQ number passed to dev_pm_set_wake_irq and dev_pm_set_dedicated_wake_irq is valid (not negative) before accepting it.

Signed-off-by Dmitry Torokhov <[email protected]>

---

My recent change to i2c core introduced a code path that led to calling dev_pm_set_wake_irq(&client->dev, -ENOENT), which succeeded but obviously did the wrong thing. Checking the IRQ and bailing out early would have helped noticing this issue earlier.

 drivers/base/power/wakeirq.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/base/power/wakeirq.c +0 -0
index eb6e674..0d77cd6 100644
@@ -68,6 +68,9 @@ int dev_pm_set_wake_irq(struct device *dev, int irq)
 	struct wake_irq *wirq;
 	int err;
Quoted 3 lines

drivers/base/power/wakeirq.c int dev_pm_set_wake_irq(struct device *dev, int irq) · line 71

Quoted 1 line
drivers/base/power/wakeirq.c +1 -0
+	if (irq < 0)

<= 0 ?

Maybe. I am still confused whether we treat 0 as invalid or not.

Quoted 22 lines
Quoted 17 lines
drivers/base/power/wakeirq.c +5 -0
+		return -EINVAL;
+
 	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
 	if (!wirq)
 		return -ENOMEM;

@@ -167,6 +170,9 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 	struct wake_irq *wirq;
 	int err;
 
+	if (irq < 0)
+		return -EINVAL;
+
 	wirq = kzalloc(sizeof(*wirq), GFP_KERNEL);
 	if (!wirq)
 		return -ENOMEM;
Signature · 2 lines
regards,
-grygorii
Signature · 1 line
Dmitry
#3 lore raw
Quoted 24 lines · Dmitry Torokhov
Quoted 23 lines · Grygorii Strashko
Quoted 22 lines · Dmitry Torokhov

Check that IRQ number passed to dev_pm_set_wake_irq and dev_pm_set_dedicated_wake_irq is valid (not negative) before accepting it.

Signed-off-by Dmitry Torokhov <[email protected]>

---

My recent change to i2c core introduced a code path that led to calling dev_pm_set_wake_irq(&client->dev, -ENOENT), which succeeded but obviously did the wrong thing. Checking the IRQ and bailing out early would have helped noticing this issue earlier.

 drivers/base/power/wakeirq.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/base/power/wakeirq.c +0 -0
index eb6e674..0d77cd6 100644
@@ -68,6 +68,9 @@ int dev_pm_set_wake_irq(struct device *dev, int irq)
 	struct wake_irq *wirq;
 	int err;
Quoted 5 lines
Quoted 3 lines

drivers/base/power/wakeirq.c int dev_pm_set_wake_irq(struct device *dev, int irq) · line 71

Quoted 1 line
drivers/base/power/wakeirq.c +1 -0
+	if (irq < 0)

<= 0 ?

Maybe. I am still confused whether we treat 0 as invalid or not.

Well, it all boils down to whether or not IRQ 0 may be a valid wakeup IRQ on any architectures.

In any case, though, we can add that check later, so I'll apply the patch as is.

Thanks,
Rafael