linux-kernel - Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO driver

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: <cf77f08d-0516-4adf-a701-9589f0d99eb5@alliedtelesis.co.nz>
Date: Tue, 17 Dec 2024 10:47:10 +1300
From: Chris Packham <chris.packham@...iedtelesis.co.nz>
To: Simon Horman <horms@...nel.org>
Cc: lee@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, tsbogend@...ha.franken.de,
 hkallweit1@...il.com, linux@...linux.org.uk, markus.stockhausen@....de,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, linux-mips@...r.kernel.org
Subject: Re: [PATCH v2 4/4] net: mdio: Add RTL9300 MDIO driver


On 17/12/2024 05:48, Simon Horman wrote:
> On Mon, Dec 16, 2024 at 04:13:46PM +1300, Chris Packham wrote:
>> Add a driver for the MDIO controller on the RTL9300 family of Ethernet
>> switches with integrated SoC. There are 4 physical SMI interfaces on the
>> RTL9300 but access is done using the switch ports so a single MDIO bus
>> is presented to the rest of the system.
>>
>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ...
>
>> diff --git a/drivers/net/mdio/mdio-realtek-rtl.c b/drivers/net/mdio/mdio-realtek-rtl.c
> ...
>
>> +#define MAX_SMI_BUSSES  4
>> +#define MAX_SMI_ADDR	0x1f
>> +
>> +struct realtek_mdio_priv {
>> +	struct regmap *regmap;
>> +	u8 smi_bus[MAX_PORTS];
>> +	u8 smi_addr[MAX_PORTS];
>> +	bool smi_bus_isc45[MAX_SMI_BUSSES];
>> +	u32 reg_base;
>> +};
> ...
>
>> +static int realtek_mdiobus_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct realtek_mdio_priv *priv;
>> +	struct fwnode_handle *child;
>> +	struct mii_bus *bus;
>> +	int err;
>> +
>> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*priv));
>> +	if (!bus)
>> +		return -ENOMEM;
>> +
>> +	bus->name = "Reaktek Switch MDIO Bus";
>> +	bus->read = realtek_mdio_read_c22;
>> +	bus->write = realtek_mdio_write_c22;
>> +	bus->read_c45 = realtek_mdio_read_c45;
>> +	bus->write_c45 =  realtek_mdio_write_c45;
>> +	bus->parent = dev;
>> +	priv = bus->priv;
>> +
>> +	priv->regmap = syscon_node_to_regmap(dev->parent->of_node);
>> +	if (IS_ERR(priv->regmap))
>> +		return PTR_ERR(priv->regmap);
>> +
>> +	err = device_property_read_u32(dev, "reg", &priv->reg_base);
>> +	if (err)
>> +		return err;
>> +
>> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
>> +
>> +	device_for_each_child_node(dev, child) {
>> +		u32 pn, smi_addr[2];
>> +
>> +		err = fwnode_property_read_u32(child, "reg", &pn);
>> +		if (err)
>> +			return err;
>> +
>> +		if (pn >= MAX_PORTS)
>> +			return dev_err_probe(dev, -EINVAL, "illegal port number %d\n", pn);
>> +
>> +		err = fwnode_property_read_u32_array(child, "realtek,smi-address", smi_addr, 2);
>> +		if (err) {
>> +			smi_addr[0] = 0;
>> +			smi_addr[1] = pn;
>> +		}
>> +
>> +		if (smi_addr[0] > MAX_SMI_BUSSES)
> Hi Chris,
>
> Should this condition be
>
> 		if (smi_addr[0] >= MAX_SMI_BUSSES)
Yes. You are correct.
>> +			return dev_err_probe(dev, -EINVAL, "illegal smi bus number %d\n",
>> +					     smi_addr[0]);
>> +
>> +		if (smi_addr[1] > MAX_SMI_ADDR)
>> +			return dev_err_probe(dev, -EINVAL, "illegal smi addr %d\n", smi_addr[1]);
>> +
>> +		if (fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"))
>> +			priv->smi_bus_isc45[smi_addr[0]] = true;
> Otherwise it seems that smi_bus_isc45 may overflow here.
>
> Flagged by Smatch.

Sounds like something I should start looking at for myself. Have you got 
a link to share?

>> +
>> +		priv->smi_bus[pn] = smi_addr[0];
>> +		priv->smi_addr[pn] = smi_addr[1];
>> +	}
>> +
>> +	err = realtek_mdiobus_init(priv);
>> +	if (err)
>> +		return dev_err_probe(dev, err, "failed to initialise MDIO bus controller\n");
>> +
>> +	err = devm_of_mdiobus_register(dev, bus, dev->of_node);
>> +	if (err)
>> +		return dev_err_probe(dev, err, "cannot register MDIO bus\n");
>> +
>> +	return 0;
>> +}
> ...
>