netdev - RE: [PATCH 1/2] gianfar: Deal with link state changes during GFAR

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] [day] [month] [year] [list]

Date:   Thu, 16 Feb 2017 17:41:47 +0000
From:   Claudiu Manoil <claudiu.manoil@....com>
To:     Thomas Graziadei <thomas.graziadei@...cronenergy.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH 1/2] gianfar: Deal with link state changes during
 GFAR_RESETTING dev state

>-----Original Message-----
>From: Thomas Graziadei [mailto:thomas.graziadei@...cronenergy.com]
>Sent: Monday, February 13, 2017 2:22 PM
>To: claudiu.manoil@...escale.com; netdev@...r.kernel.org; linux-
>kernel@...r.kernel.org
>Cc: Thomas Graziadei <thomas.graziadei@...cronenergy.com>
>Subject: [PATCH 1/2] gianfar: Deal with link state changes during GFAR_RESETTING
>dev state
>
>From: Thomas Graziadei <thomas.graziadei@...cronenergy.com>
>

[...]

>
>To resolve the issue adjust_link is called after every GFAR_RESETTING lock
>section. Adjust_link itself checks if anything has changed and updates the
>link accordingly.
>

Calling adjust_link() from the ethernet driver, even if only following device reset / 
reconfig sections, is racy - the phylib state machine may run adjust_link() at the 
same time or the phydev state may be inconsistent - and may end up in more 
subtle link state issues.  It also defeats the purpose of the adjust link hook, which 
is to be called by the phylib state machine.

I had a look at this, and I don't think there is an easy fix in the driver for your case. 
However, the workaround should be straight forward: wait for the link to stabilize 
to the requested parameters before applying other device configuration changes 
that require device reset (like rx timestamping).  Given that the PHYLIB state machine 
changed its behavior since adjust_link() in gianfar was last modified (i.e. it used to be called
periodically before, from RUNNING<->CHANGELINK states) and given that there is a good part 
of legacy code in the current adjust_link() implementation, I think that driver part needs 
considerable rework to correctly cover this usecase as well.

Thanks.
Claudiu