From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750999AbdAXUwN (ORCPT ); Tue, 24 Jan 2017 15:52:13 -0500 Received: from mail-pf0-f196.google.com ([209.85.192.196]:34217 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750951AbdAXUwJ (ORCPT ); Tue, 24 Jan 2017 15:52:09 -0500 Subject: Re: [PATCH 1/2] net: phy: leds: Clear phy_num_led_triggers on failure to avoid crash To: Geert Uytterhoeven , "David S. Miller" , Zach Brown , Volodymyr Bendiuga References: <1485272470-3497-1-git-send-email-geert+renesas@glider.be> <1485272470-3497-2-git-send-email-geert+renesas@glider.be> Cc: Andrew Lunn , =?UTF-8?Q?Magnus_=c3=96berg?= , netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org From: Florian Fainelli Message-ID: <858caf82-b49e-96d9-98a5-32be915c3f74@gmail.com> Date: Tue, 24 Jan 2017 12:52:02 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1485272470-3497-2-git-send-email-geert+renesas@glider.be> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/24/2017 07:41 AM, Geert Uytterhoeven wrote: > phy_attach_direct() ignores errors returned by > phy_led_triggers_register(). I think that's OK, as LED triggers can be > considered a non-critical feature. > > However, this causes problems later: > - phy_led_trigger_change_speed() will access the array > phy_device.phy_led_triggers, which has been freed in the error path > of phy_led_triggers_register(), which may lead to a crash. > > - phy_led_triggers_unregister() will access the same array, leading to > crashes during s2ram or poweroff, like: > > Unable to handle kernel NULL pointer dereference at virtual address > 00000000 > ... > [] (__list_del_entry_valid) from [] (led_trigger_unregister+0x34/0xcc) > [] (led_trigger_unregister) from [] (phy_led_triggers_unregister+0x28/0x34) > [] (phy_led_triggers_unregister) from [] (phy_detach+0x30/0x74) > [] (phy_detach) from [] (sh_eth_close+0x64/0x9c) > [] (sh_eth_close) from [] (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! > ... > [] (__list_del_entry_valid) from [] (led_trigger_unregister+0x34/0xcc) > [] (led_trigger_unregister) from [] (phy_led_triggers_unregister+0x28/0x34) > [] (phy_led_triggers_unregister) from [] (phy_detach+0x30/0x74) > [] (phy_detach) from [] (sh_eth_close+0x6c/0xa4) > [] (sh_eth_close) from [] (__dev_close_many+0xac/0xd0) > > To fix this, clear phy_device.phy_num_led_triggers in the error path of > phy_led_triggers_register() fails. > > Note that the "No phy led trigger registered for speed" message will > still be printed on link speed changes, which is a good cue that > something went wrong with the LED triggers. > > Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy link state change") > Signed-off-by: Geert Uytterhoeven Reviewed-by: Florian Fainelli -- Florian