netdev - Re: [PATCH net] net: dsa: Ensure validity of dst->ds[0]

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, 09 Jan 2017 15:50:53 -0500
From:   Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:     Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, andrew@...n.ch,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net] net: dsa: Ensure validity of dst->ds[0]

Hi Florian,

Florian Fainelli <f.fainelli@...il.com> writes:

> It is perfectly possible to have non zero indexed switches being present
> in a DSA switch tree, in such a case, we will be deferencing a NULL
> pointer while dsa_cpu_port_ethtool_{setup,restore}. Be more defensive
> and ensure that dst->ds[0] is valid before doing anything with it.
>
> Fixes: 0c73c523cf73 ("net: dsa: Initialize CPU port ethtool ops per tree")
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>

Reviewed-by: Vivien Didelot <vivien.didelot@...oirfairelinux.com>

The patch is correct since we are already using dst->ds[0] here.

But we should stop using that and use dst->cpu_switch instead, because
the switch with ID 0 won't necessary be the CPU switch. Now that the
Ethernet switch chips are true Linux devices, they are registered in
order depending on their bus/address. So in a setup like this:

       ,--MDIO--@...------@2--
      |         |         |
    [CPU] <-> [swA] <-> [swB]

swB will have index 0 and swA will have index 1. Please correct me if
I'm wrong.

Thanks,

        Vivien