netdev - Re: [PATCH v3 7/9] net: mvmdio: add xmdio xsmi support

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]

Message-ID: <20170612100756.GZ4902@n2100.armlinux.org.uk>
Date:   Mon, 12 Jun 2017 11:07:56 +0100
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Antoine Tenart <antoine.tenart@...e-electrons.com>
Cc:     davem@...emloft.net, jason@...edaemon.net, andrew@...n.ch,
        gregory.clement@...e-electrons.com,
        sebastian.hesselbarth@...il.com, f.fainelli@...il.com,
        thomas.petazzoni@...e-electrons.com, nadavh@...vell.com,
        mw@...ihalf.com, netdev@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 7/9] net: mvmdio: add xmdio xsmi support

On Mon, Jun 12, 2017 at 11:57:43AM +0200, Antoine Tenart wrote:
> +static void orion_mdio_xsmi_start_read_op(struct orion_mdio_dev *dev,
> +					  int mii_id, int regnum)
> +{
> +	u16 dev_addr = (regnum >> 16) & GENMASK(4, 0);
> +
> +	writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG);
> +	writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) |
> +	       (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) |
> +	       MVMDIO_XSMI_READ_OPERATION,
> +	       dev->regs + MVMDIO_XSMI_MGNT_REG);

So what happens if this function gets passed a Clause 22 formatted
request?  Both regnum and mii_id are in the range 0-31.  MII_ADDR_C45
in regnum is clear.

The answer is we produce two Clause 45 frames on the bus, which is
certainly not correct.  You need to trap and error out if MII_ADDR_C45
is not set (as I've already said previously.)

The SMI operations need to do the reverse - they need to fail if they
receive a regnum with MII_ADDR_C45 set, as they are unable to produce
Clause 45 frames.

> +static void orion_mdio_xsmi_write_op(struct orion_mdio_dev *dev, int mii_id,
> +				     int regnum, u16 value)
> +{
> +	u16 dev_addr = (regnum >> 16) & GENMASK(4, 0);
> +
> +	writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG);
> +	writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) |
> +	       (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) |
> +	       MVMDIO_XSMI_WRITE_OPERATION | value,
> +	       dev->regs + MVMDIO_XSMI_MGNT_REG);

It's even more important here, as a Clause 22 write will produce a
valid Clause 45 pair of frames with dev_addr = 0.

Again, the corresponding SMI code needs to reject Clause 45 requests
as the SMI interface is unable to produce Clause 45 frames.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.