netdev - Re: [RFC PATCH] net: mvpp2: fix detection of 10G SFP modules

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87efb37jhr.fsf@tkos.co.il>
Date:   Thu, 29 Nov 2018 21:48:32 +0200
From:   Baruch Siach <baruch@...s.co.il>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        Antoine Tenart <antoine.tenart@...tlin.com>
Subject: Re: [RFC PATCH] net: mvpp2: fix detection of 10G SFP modules

Hi Florian,

Florian Fainelli writes:
> On 11/29/2018 4:49 AM, Baruch Siach wrote:
>> The mvpp2_phylink_validate() relies on the interface field of
>> phylink_link_state to determine valid link modes. However, when called
>> from phylink_sfp_module_insert() this field in not initialized. The
>> default switch case then excludes 10G link modes. This allows 10G SFP
>> modules that are detected correctly to be configured at max rate of
>> 2.5G.
>>
>> Catch the uninitialized PHY mode case, and allow 10G rates.
>>
>> Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>
>> Cc: Antoine Tenart <antoine.tenart@...tlin.com>
>> Signed-off-by: Baruch Siach <baruch@...s.co.il>
>> ---
>> Is that the right fix?
>
> It would be a bit surprising that this is the right fix, you would
> expect validate to be called once everything has been parsed
> successfully from the SFP, is not that the case here? If not, can you
> find out what happens?

This is the code from phylink_sfp_module_insert() at
drivers/net/phy/phylink.c:

        __ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, };
        struct phylink_link_state config;
        ...

        sfp_parse_support(pl->sfp_bus, id, support);
        port = sfp_parse_port(pl->sfp_bus, id, support);

        memset(&config, 0, sizeof(config));
        linkmode_copy(config.advertising, support);
        config.interface = PHY_INTERFACE_MODE_NA;
        /* ... more 'config' fields initialization ... */
        ret = phylink_validate(pl, support, &config);

The 'interface' field is not detected at this stage. I think it is not
meant to represent the detected information, but the actual
configuration.

baruch

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -