Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1566473 > unrolled thread
| Started by | Geert Uytterhoeven <geert+renesas@glider.be> |
|---|---|
| First post | 2017-01-25 11:50 +0100 |
| Last post | 2017-01-25 20:50 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/3] net: phy: leds: Fix truncated LED trigger names and crashes Geert Uytterhoeven <geert+renesas@glider.be> - 2017-01-25 11:50 +0100
[PATCH v2 3/3] net: phy: leds: Fix truncated LED trigger names Geert Uytterhoeven <geert+renesas@glider.be> - 2017-01-25 11:50 +0100
Re: [PATCH v2 3/3] net: phy: leds: Fix truncated LED trigger names Andrew Lunn <andrew@lunn.ch> - 2017-01-25 14:00 +0100
Re: [PATCH v2 0/3] net: phy: leds: Fix truncated LED trigger names and crashes David Miller <davem@davemloft.net> - 2017-01-25 20:50 +0100
| From | Geert Uytterhoeven <geert+renesas@glider.be> |
|---|---|
| Date | 2017-01-25 11:50 +0100 |
| Subject | [PATCH v2 0/3] net: phy: leds: Fix truncated LED trigger names and crashes |
| Message-ID | <t3tjk-Dr-5@gated-at.bofh.it> |
Hi David,
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>.
Thanks!
Geert Uytterhoeven (3):
net: phy: leds: Clear phy_num_led_triggers on failure to avoid crash
net: phy: leds: Break dependency of phy.h on phy_led_triggers.h
net: phy: leds: Fix truncated LED trigger names
drivers/net/phy/phy.c | 1 +
drivers/net/phy/phy_led_triggers.c | 9 +++++++--
include/linux/phy.h | 1 -
include/linux/phy_led_triggers.h | 4 ++--
4 files changed, 10 insertions(+), 5 deletions(-)
--
1.9.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [next] | [standalone]
| From | Geert Uytterhoeven <geert+renesas@glider.be> |
|---|---|
| Date | 2017-01-25 11:50 +0100 |
| Subject | [PATCH v2 3/3] net: phy: leds: Fix truncated LED trigger names |
| Message-ID | <t3tjk-Dr-21@gated-at.bofh.it> |
| In reply to | #1566473 |
Commit 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and
bus_id") increased the size of MII bus IDs, but forgot to update the
private definition in <linux/phy_led_triggers.h>.
This may cause:
1. Truncation of LED trigger names,
2. Duplicate LED trigger names,
3. Failures registering LED triggers,
4. Crashes due to bad error handling in the LED trigger failure path.
To fix this, and prevent the definitions going out of sync again in the
future, let the PHY LED trigger code use the existing MII_BUS_ID_SIZE
definition.
Example:
- Before I had triggers "ee700000.etherne:01:100Mbps" and
"ee700000.etherne:01:10Mbps",
- After the increase of MII_BUS_ID_SIZE, both became
"ee700000.ethernet-ffffffff:01:" => FAIL,
- Now, the triggers are "ee700000.ethernet-ffffffff:01:100Mbps" and
"ee700000.ethernet-ffffffff:01:10Mbps", which are unique again.
Fixes: 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and bus_id")
Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy link state change")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
- 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>.
---
include/linux/phy_led_triggers.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h
index a2daea0a37d2ae14..b37b05bfd1a6dd8a 100644
--- a/include/linux/phy_led_triggers.h
+++ b/include/linux/phy_led_triggers.h
@@ -18,11 +18,11 @@
#ifdef CONFIG_LED_TRIGGER_PHY
#include <linux/leds.h>
+#include <linux/phy.h>
#define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE 10
-#define PHY_MII_BUS_ID_SIZE (20 - 3)
-#define PHY_LINK_LED_TRIGGER_NAME_SIZE (PHY_MII_BUS_ID_SIZE + \
+#define PHY_LINK_LED_TRIGGER_NAME_SIZE (MII_BUS_ID_SIZE + \
FIELD_SIZEOF(struct mdio_device, addr)+\
PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE)
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Andrew Lunn <andrew@lunn.ch> |
|---|---|
| Date | 2017-01-25 14:00 +0100 |
| Subject | Re: [PATCH v2 3/3] net: phy: leds: Fix truncated LED trigger names |
| Message-ID | <t3vl8-1RZ-11@gated-at.bofh.it> |
| In reply to | #1566474 |
On Wed, Jan 25, 2017 at 11:39:50AM +0100, Geert Uytterhoeven wrote:
> Commit 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and
> bus_id") increased the size of MII bus IDs, but forgot to update the
> private definition in <linux/phy_led_triggers.h>.
> This may cause:
> 1. Truncation of LED trigger names,
> 2. Duplicate LED trigger names,
> 3. Failures registering LED triggers,
> 4. Crashes due to bad error handling in the LED trigger failure path.
>
> To fix this, and prevent the definitions going out of sync again in the
> future, let the PHY LED trigger code use the existing MII_BUS_ID_SIZE
> definition.
>
> Example:
> - Before I had triggers "ee700000.etherne:01:100Mbps" and
> "ee700000.etherne:01:10Mbps",
> - After the increase of MII_BUS_ID_SIZE, both became
> "ee700000.ethernet-ffffffff:01:" => FAIL,
> - Now, the triggers are "ee700000.ethernet-ffffffff:01:100Mbps" and
> "ee700000.ethernet-ffffffff:01:10Mbps", which are unique again.
>
> Fixes: 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and bus_id")
> Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy link state change")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2017-01-25 20:50 +0100 |
| Subject | Re: [PATCH v2 0/3] net: phy: leds: Fix truncated LED trigger names and crashes |
| Message-ID | <t3BJT-5ZU-7@gated-at.bofh.it> |
| In reply to | #1566473 |
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 25 Jan 2017 11:39:47 +0100
> 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.
Thanks for tracking this down and fixing it.
Series applied, thanks.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web