netdev - Re: [RFC net-next 4/4] net: phy: Correctly process PHY_HALTED in phy_stop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Oct 2017 09:33:14 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Marc Gonzalez <marc_gonzalez@...madesigns.com>,
        "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>, opendmb@...il.com,
        Mason <slash.tmp@...e.fr>, David Daney <david.daney@...ium.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>
Subject: Re: [RFC net-next 4/4] net: phy: Correctly process PHY_HALTED in
 phy_stop_machine()

On 10/31/2017 08:26 AM, Geert Uytterhoeven wrote:
> Hi Florian,
> 
> On Mon, Oct 30, 2017 at 5:09 PM, Florian Fainelli <f.fainelli@...il.com> wrote:
>> On 10/30/2017 06:56 AM, Geert Uytterhoeven wrote:
>>> On Thu, Oct 26, 2017 at 1:21 AM, Florian Fainelli <f.fainelli@...il.com> wrote:
>>>> Marc reported that he was not getting the PHY library adjust_link()
>>>> callback function to run when calling phy_stop() + phy_disconnect()
>>>> which does not indeed happen because we set the state machine to
>>>> PHY_HALTED but we don't get to run it to process this state past that
>>>> point.
>>>>
>>>> Fix this with a synchronous call to phy_state_machine() in order to have
>>>> the state machine actually act on PHY_HALTED, set the PHY device's link
>>>> down, turn the network device's carrier off and finally call the
>>>> adjust_link() function.
>>>>
>>>> At the end of phy_state_machine() though, if we are going to be moving
>>>> from PHY_HALTED to PHY_HALTED, do not reschedule the state machine, this
>>>> is pointless.
>>>>
>>>> Reported-by: Marc Gonzalez <marc_gonzalez@...madesigns.com>
>>>> Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@...madesigns.com>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>>>
>>> Thanks for your patch!
>>>
>>> Unfortunately, after applying this one, the last in your series, both
>>> sh73a0/kzm9g and r8a73a4/ape6evm start crashing again in the system
>>> suspend/resume path, due to register accesses while the device is already
>>> suspended:
>>
>> OK, seems like there is another path, uncovered by this patch that we
>> can be hitting, does the following patch below help?
> 
> Unfortunately it doesn't help.

OK :/

> 
>>> Unhandled fault: imprecise external abort (0x1406) at 0x0005b950
> 
> Note that this is an imprecise external abort, i.e. it's reporting may
> be delayed,
> and the backtrace may be inaccurate.

True, can you help narrow it down with me? Can you confirm that
adjust_link() (assuming that is the problem) does not get called past
phy_stop_machine() as it should?
-- 
Florian