netdev - Re: [PATCH v3 2/2] net: phy: add marvell usb to mdio controller

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: <20200322074006.GB64528@kroah.com>
Date:   Sun, 22 Mar 2020 08:40:06 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Tobias Waldekranz <tobias@...dekranz.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch,
        f.fainelli@...il.com, hkallweit1@...il.com,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH v3 2/2] net: phy: add marvell usb to mdio controller

On Sat, Mar 21, 2020 at 09:24:43PM +0100, Tobias Waldekranz wrote:
> An MDIO controller present on development boards for Marvell switches
> from the Link Street (88E6xxx) family.
> 
> Using this module, you can use the following setup as a development
> platform for switchdev and DSA related work.
> 
>    .-------.      .-----------------.
>    |      USB----USB                |
>    |  SoC  |      |  88E6390X-DB  ETH1-10
>    |      ETH----ETH0               |
>    '-------'      '-----------------'
> 
> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
> ---
> 
> Hi linux-usb,
> 
> This is my first ever USB driver, therefore I would really appreciate
> it if someone could have a look at it from a USB perspective before it
> is (hopefully) pulled into net-next.

>From a USB point of view, it looks sane, only one question:

> +static int mvusb_mdio_probe(struct usb_interface *interface,
> +			    const struct usb_device_id *id)
> +{
> +	struct device *dev = &interface->dev;
> +	struct mvusb_mdio *mvusb;
> +	struct mii_bus *mdio;
> +
> +	mdio = devm_mdiobus_alloc_size(dev, sizeof(*mvusb));

You allocate a bigger buffer here than the original pointer thinks it is
pointing to?

> +	if (!mdio)
> +		return -ENOMEM;
> +
> +	mvusb = mdio->priv;

And then you set this pointer here?

If that's the way this is supposed to work, that's fine, just feels like
the math is wrong somewhere...

thanks,

greg k-h