netdev - Re: [PATCH v9 2/2] net: ti: icssg-prueth: Add ICSSG ethernet 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: <78b82c086c91be61d6a15582a7dc6f52b92f1b3e.camel@redhat.com>
Date: Tue, 18 Jul 2023 10:22:53 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: MD Danish Anwar <danishanwar@...com>, Randy Dunlap
 <rdunlap@...radead.org>,  Roger Quadros <rogerq@...nel.org>, Simon Horman
 <simon.horman@...igine.com>, Vignesh Raghavendra <vigneshr@...com>, Andrew
 Lunn <andrew@...n.ch>, Richard Cochran <richardcochran@...il.com>, Conor
 Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Rob Herring <robh+dt@...nel.org>,
 Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
 "David S. Miller" <davem@...emloft.net>
Cc: nm@...com, srk@...com, linux-kernel@...r.kernel.org, 
 devicetree@...r.kernel.org, netdev@...r.kernel.org,
 linux-omap@...r.kernel.org,  linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v9 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

On Fri, 2023-07-14 at 15:14 +0530, MD Danish Anwar wrote:
> +static int prueth_netdev_init(struct prueth *prueth,
> +			      struct device_node *eth_node)
> +{
> +	int ret, num_tx_chn = PRUETH_MAX_TX_QUEUES;
> +	struct prueth_emac *emac;
> +	struct net_device *ndev;
> +	enum prueth_port port;
> +	enum prueth_mac mac;
> +
> +	port = prueth_node_port(eth_node);
> +	if (port == PRUETH_PORT_INVALID)
> +		return -EINVAL;
> +
> +	mac = prueth_node_mac(eth_node);
> +	if (mac == PRUETH_MAC_INVALID)
> +		return -EINVAL;
> +
> +	ndev = alloc_etherdev_mq(sizeof(*emac), num_tx_chn);
> +	if (!ndev)
> +		return -ENOMEM;
> +
> +	emac = netdev_priv(ndev);
> +	prueth->emac[mac] = emac;
> +	emac->prueth = prueth;
> +	emac->ndev = ndev;
> +	emac->port_id = port;
> +	emac->cmd_wq = create_singlethread_workqueue("icssg_cmd_wq");
> +	if (!emac->cmd_wq) {
> +		ret = -ENOMEM;
> +		goto free_ndev;
> +	}
> +	INIT_WORK(&emac->rx_mode_work, emac_ndo_set_rx_mode_work);
> +
> +	ret = pruss_request_mem_region(prueth->pruss,
> +				       port == PRUETH_PORT_MII0 ?
> +				       PRUSS_MEM_DRAM0 : PRUSS_MEM_DRAM1,
> +				       &emac->dram);
> +	if (ret) {
> +		dev_err(prueth->dev, "unable to get DRAM: %d\n", ret);
> +		ret = -ENOMEM;
> +		goto free_wq;
> +	}
> +
> +	emac->tx_ch_num = 1;
> +
> +	SET_NETDEV_DEV(ndev, prueth->dev);
> +	spin_lock_init(&emac->lock);
> +	mutex_init(&emac->cmd_lock);
> +
> +	emac->phy_node = of_parse_phandle(eth_node, "phy-handle", 0);
> +	if (!emac->phy_node && !of_phy_is_fixed_link(eth_node)) {
> +		dev_err(prueth->dev, "couldn't find phy-handle\n");
> +		ret = -ENODEV;
> +		goto free;
> +	} else if (of_phy_is_fixed_link(eth_node)) {
> +		ret = of_phy_register_fixed_link(eth_node);
> +		if (ret) {
> +			ret = dev_err_probe(prueth->dev, ret,
> +					    "failed to register fixed-link phy\n");
> +			goto free;
> +		}
> +
> +		emac->phy_node = eth_node;
> +	}
> +
> +	ret = of_get_phy_mode(eth_node, &emac->phy_if);
> +	if (ret) {
> +		dev_err(prueth->dev, "could not get phy-mode property\n");
> +		goto free;
> +	}
> +
> +	if (emac->phy_if != PHY_INTERFACE_MODE_MII &&
> +	    !phy_interface_mode_is_rgmii(emac->phy_if)) {
> +		dev_err(prueth->dev, "PHY mode unsupported %s\n", phy_modes(emac->phy_if));
> +		ret = -EINVAL;
> +		goto free;
> +	}
> +
> +	/* AM65 SR2.0 has TX Internal delay always enabled by hardware
> +	 * and it is not possible to disable TX Internal delay. The below
> +	 * switch case block describes how we handle different phy modes
> +	 * based on hardware restriction.
> +	 */
> +	switch (emac->phy_if) {
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +		emac->phy_if = PHY_INTERFACE_MODE_RGMII_RXID;
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		emac->phy_if = PHY_INTERFACE_MODE_RGMII;
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +		dev_err(prueth->dev, "RGMII mode without TX delay is not supported");
> +		return -EINVAL;

At this point ndev prueth->emac[mac] == emac, so the caller will try to
clean it up via prueth_netdev_exit(), which in turn expects the device
being fully initialized, while this is not. Notably the napi instance
has not being registered yet.

You should 'goto free;' above and possibly move the 'ndev prueth-
>emac[mac] = emac' assignment at the end of this function.

Thanks!

Paolo