netdev - Re: Regression: supported_interfaces filling enforcement

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: Mon, 10 Jul 2023 15:35:56 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Sergei Antonov <saproj@...il.com>
Cc: netdev@...r.kernel.org, rmk+kernel@...linux.org.uk
Subject: Re: Regression: supported_interfaces filling enforcement

On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> Hello!
> This commit seems to break the mv88e6060 dsa driver:
> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> supported_interfaces to be filled"
> 
> The driver does not fill 'supported_interfaces'. What is the proper
> way to fix it? I managed to fix it by the following quick code.
> Comments? Recommendations?

Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
supported_interfaces to be filled") was based on a miscalculation.

> 
> +static void mv88e6060_get_caps(struct dsa_switch *ds, int port,
> +                              struct phylink_config *config)
> +{
> +       __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> +       __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces);

This is enough to fix phylink generic validation on the front-facing
ports with internal PHYs. But it is possible (and encouraged) to use
phylink on the CPU port too (rev-mii, rev-rmii); currently that's not
enforced for mv88e6060 because it's in the dsa_switches_apply_workarounds[]
array.

Could you please modify your device tree to add a fixed-link and
phy-mode property on your CPU port so that phylink does get used, and
populate supported_interfaces and mac_capabilities properly on the MII
ports (4 and 5) as well (so that it doesn't fail validation)?

> +}
> +
>  static const struct dsa_switch_ops mv88e6060_switch_ops = {
>         .get_tag_protocol = mv88e6060_get_tag_protocol,
>         .setup          = mv88e6060_setup,
>         .phy_read       = mv88e6060_phy_read,
>         .phy_write      = mv88e6060_phy_write,
> +       .phylink_get_caps = mv88e6060_get_caps

Comma at the end, please.

>  };