diff options
author | David S. Miller <davem@davemloft.net> | 2017-01-25 14:40:25 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2017-01-25 14:40:25 -0500 |
commit | d5bdc021ecc8b273259a02ff83ab13b2de9b9717 (patch) | |
tree | 4c2a2b25a501d08ac1c9ac0381e963a3c725137b | |
parent | 8b901f6bbcf12a20e43105d161bedde093431e61 (diff) | |
parent | 3c880eb0205222bb062970085ebedc73ec8dfd14 (diff) |
Merge branch 'phy-truncated-led-names'
Geert Uytterhoeven says:
====================
net: phy: leds: Fix truncated LED trigger names and crashes
I started seeing crashes during s2ram and poweroff on all my ARM boards,
like:
Unable to handle kernel NULL pointer dereference at virtual address 00000000
...
[<c04116d4>] (__list_del_entry_valid) from [<c05e8948>] (led_trigger_unregister+0x34/0xcc)
[<c05e8948>] (led_trigger_unregister) from [<c05336c4>] (phy_led_triggers_unregister+0x28/0x34)
[<c05336c4>] (phy_led_triggers_unregister) from [<c0531d44>] (phy_detach+0x30/0x74)
[<c0531d44>] (phy_detach) from [<c0538bdc>] (sh_eth_close+0x64/0x9c)
[<c0538bdc>] (sh_eth_close) from [<c04d4ce0>] (dpm_run_callback+0x48/0xc8)
or:
list_del corruption. prev->next should be dede6540, but was 2e323931
------------[ cut here ]------------
kernel BUG at lib/list_debug.c:52!
...
[<c02f6d70>] (__list_del_entry_valid) from [<c0425168>] (led_trigger_unregister+0x34/0xcc)
[<c0425168>] (led_trigger_unregister) from [<c03a05a0>] (phy_led_triggers_unregister+0x28/0x34)
[<c03a05a0>] (phy_led_triggers_unregister) from [<c039ec04>] (phy_detach+0x30/0x74)
[<c039ec04>] (phy_detach) from [<c03a4fc0>] (sh_eth_close+0x6c/0xa4)
[<c03a4fc0>] (sh_eth_close) from [<c0483234>] (__dev_close_many+0xac/0xd0)
As the only clue was a kernel message like
sh-eth ee700000.ethernet eth0: No phy led trigger registered for speed(100)
I had to bisected this, leading to commit 4567d686f5c6d955 ("phy:
increase size of MII_BUS_ID_SIZE and bus_id"). Reverting that commit
fixed the issue.
More investigation revealed the crashes are due to the combination of
two things:
- Truncated LED trigger names, leading to duplicate names, and
registration failures,
- Bad error handling in case of registration failures.
Both are fixed by this patch series.
Changes compared to v1:
- Add Reviewed-by,
- New patch "net: phy: leds: Break dependency of phy.h on
phy_led_triggers.h",
- Drop moving the include of <linux/phy_led_triggers.h>, as
<linux/phy.h> no longer includes it,
- #include <linux/phy.h> from <linux/phy_led_triggers.h>.
====================
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/net/phy/phy.c | 1 | ||||
-rw-r--r-- | drivers/net/phy/phy_led_triggers.c | 9 | ||||
-rw-r--r-- | include/linux/phy.h | 1 | ||||
-rw-r--r-- | include/linux/phy_led_triggers.h | 4 |
4 files changed, 10 insertions, 5 deletions
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index e687a9cb4a37..7cc1b7dcfe05 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c | |||
@@ -29,6 +29,7 @@ | |||
29 | #include <linux/mii.h> | 29 | #include <linux/mii.h> |
30 | #include <linux/ethtool.h> | 30 | #include <linux/ethtool.h> |
31 | #include <linux/phy.h> | 31 | #include <linux/phy.h> |
32 | #include <linux/phy_led_triggers.h> | ||
32 | #include <linux/timer.h> | 33 | #include <linux/timer.h> |
33 | #include <linux/workqueue.h> | 34 | #include <linux/workqueue.h> |
34 | #include <linux/mdio.h> | 35 | #include <linux/mdio.h> |
diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c index fa62bdf2f526..94ca42e630bb 100644 --- a/drivers/net/phy/phy_led_triggers.c +++ b/drivers/net/phy/phy_led_triggers.c | |||
@@ -12,6 +12,7 @@ | |||
12 | */ | 12 | */ |
13 | #include <linux/leds.h> | 13 | #include <linux/leds.h> |
14 | #include <linux/phy.h> | 14 | #include <linux/phy.h> |
15 | #include <linux/phy_led_triggers.h> | ||
15 | #include <linux/netdevice.h> | 16 | #include <linux/netdevice.h> |
16 | 17 | ||
17 | static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy, | 18 | static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy, |
@@ -102,8 +103,10 @@ int phy_led_triggers_register(struct phy_device *phy) | |||
102 | sizeof(struct phy_led_trigger) * | 103 | sizeof(struct phy_led_trigger) * |
103 | phy->phy_num_led_triggers, | 104 | phy->phy_num_led_triggers, |
104 | GFP_KERNEL); | 105 | GFP_KERNEL); |
105 | if (!phy->phy_led_triggers) | 106 | if (!phy->phy_led_triggers) { |
106 | return -ENOMEM; | 107 | err = -ENOMEM; |
108 | goto out_clear; | ||
109 | } | ||
107 | 110 | ||
108 | for (i = 0; i < phy->phy_num_led_triggers; i++) { | 111 | for (i = 0; i < phy->phy_num_led_triggers; i++) { |
109 | err = phy_led_trigger_register(phy, &phy->phy_led_triggers[i], | 112 | err = phy_led_trigger_register(phy, &phy->phy_led_triggers[i], |
@@ -120,6 +123,8 @@ out_unreg: | |||
120 | while (i--) | 123 | while (i--) |
121 | phy_led_trigger_unregister(&phy->phy_led_triggers[i]); | 124 | phy_led_trigger_unregister(&phy->phy_led_triggers[i]); |
122 | devm_kfree(&phy->mdio.dev, phy->phy_led_triggers); | 125 | devm_kfree(&phy->mdio.dev, phy->phy_led_triggers); |
126 | out_clear: | ||
127 | phy->phy_num_led_triggers = 0; | ||
123 | return err; | 128 | return err; |
124 | } | 129 | } |
125 | EXPORT_SYMBOL_GPL(phy_led_triggers_register); | 130 | EXPORT_SYMBOL_GPL(phy_led_triggers_register); |
diff --git a/include/linux/phy.h b/include/linux/phy.h index f7d95f644eed..7fc1105605bf 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h | |||
@@ -25,7 +25,6 @@ | |||
25 | #include <linux/timer.h> | 25 | #include <linux/timer.h> |
26 | #include <linux/workqueue.h> | 26 | #include <linux/workqueue.h> |
27 | #include <linux/mod_devicetable.h> | 27 | #include <linux/mod_devicetable.h> |
28 | #include <linux/phy_led_triggers.h> | ||
29 | 28 | ||
30 | #include <linux/atomic.h> | 29 | #include <linux/atomic.h> |
31 | 30 | ||
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h index a2daea0a37d2..b37b05bfd1a6 100644 --- a/include/linux/phy_led_triggers.h +++ b/include/linux/phy_led_triggers.h | |||
@@ -18,11 +18,11 @@ struct phy_device; | |||
18 | #ifdef CONFIG_LED_TRIGGER_PHY | 18 | #ifdef CONFIG_LED_TRIGGER_PHY |
19 | 19 | ||
20 | #include <linux/leds.h> | 20 | #include <linux/leds.h> |
21 | #include <linux/phy.h> | ||
21 | 22 | ||
22 | #define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 10 | 23 | #define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 10 |
23 | #define PHY_MII_BUS_ID_SIZE (20 - 3) | ||
24 | 24 | ||
25 | #define PHY_LINK_LED_TRIGGER_NAME_SIZE (PHY_MII_BUS_ID_SIZE + \ | 25 | #define PHY_LINK_LED_TRIGGER_NAME_SIZE (MII_BUS_ID_SIZE + \ |
26 | FIELD_SIZEOF(struct mdio_device, addr)+\ | 26 | FIELD_SIZEOF(struct mdio_device, addr)+\ |
27 | PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE) | 27 | PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE) |
28 | 28 | ||