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: <YuBi0+wJBlqBPhKn@lunn.ch>
Date:   Tue, 26 Jul 2022 23:55:31 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     alexandru.tachici@...log.com
Cc:     netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, devicetree@...r.kernel.org,
        krzysztof.kozlowski+dt@...aro.org, gerhard@...leder-embedded.com,
        geert+renesas@...der.be, joel@....id.au, stefan.wahren@...e.com,
        wellslutw@...il.com, geert@...ux-m68k.org, robh+dt@...nel.org,
        d.michailidis@...gible.com, stephen@...workplumber.org,
        l.stelmach@...sung.com, linux-kernel@...r.kernel.org
Subject: Re: [net-next v2 2/3] net: ethernet: adi: Add ADIN1110 support

> +static int adin1110_set_mac_address(struct net_device *netdev, const unsigned char *dev_addr)
> +{
> +	struct adin1110_port_priv *port_priv = netdev_priv(netdev);
> +	u8 mask[ETH_ALEN];
> +
> +	if (!is_valid_ether_addr(dev_addr))
> +		return -EADDRNOTAVAIL;
> +
> +	eth_hw_addr_set(netdev, dev_addr);
> +	memset(mask, 0xFF, ETH_ALEN);
> +
> +	return adin1110_write_mac_address(port_priv, ADIN_MAC_ADDR_SLOT, netdev->dev_addr, mask);

It looks like you have one slot for this? But two interfaces? So if
you change it on one, you actually change it for both? I would say
this needs handling better, either two slots, or refuse to allow it to
be changed.

> +static int adin1110_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
> +{
> +	if (!netif_running(netdev))
> +		return -EINVAL;
> +
> +	if (!netdev->phydev)
> +		return -ENODEV;
> +
> +	return phy_mii_ioctl(netdev->phydev, rq, cmd);

phy_do_ioctl()

> +static int adin1110_probe_netdevs(struct adin1110_priv *priv)
> +{
> +	struct device *dev = &priv->spidev->dev;
> +	struct adin1110_port_priv *port_priv;
> +	struct net_device *netdev;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < priv->cfg->ports_nr; i++) {
> +		netdev = devm_alloc_etherdev(dev, sizeof(*port_priv));
> +		if (!netdev)
> +			return -ENOMEM;
> +
> +		port_priv = netdev_priv(netdev);
> +		port_priv->netdev = netdev;
> +		port_priv->priv = priv;
> +		port_priv->cfg = priv->cfg;
> +		port_priv->nr = i;
> +		priv->ports[i] = port_priv;
> +		SET_NETDEV_DEV(netdev, dev);
> +
> +		ret = device_get_ethdev_address(dev, netdev);
> +		if (ret < 0)
> +			return ret;
> +
> +		netdev->irq = priv->spidev->irq;
> +		INIT_WORK(&port_priv->tx_work, adin1110_tx_work);
> +		INIT_WORK(&port_priv->rx_mode_work, adin1110_rx_mode_work);
> +		skb_queue_head_init(&port_priv->txq);
> +
> +		netif_carrier_off(netdev);
> +
> +		netdev->if_port = IF_PORT_10BASET;
> +		netdev->netdev_ops = &adin1110_netdev_ops;
> +		netdev->ethtool_ops = &adin1110_ethtool_ops;
> +		netdev->priv_flags |= IFF_UNICAST_FLT;
> +		netdev->features |= NETIF_F_NETNS_LOCAL;
> +
> +		ret = devm_register_netdev(dev, netdev);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to register network device\n");
> +			return ret;
> +		}

At this point, the device is live. If you are using NFS root for
example, the kernel will try mounting the rootfs before this even
returns. So you need to ensure your netdev is functional at this
point. Anything which happens afterwards needs to be optional, not
cause an OOPS if missing etc.

> +
> +		port_priv->phydev = get_phy_device(priv->mii_bus, i + 1, false);
> +		if (!port_priv->phydev) {
> +			netdev_err(netdev, "Could not find PHY with device address: %d.\n", i);
> +			return -ENODEV;
> +		}
> +
> +		port_priv->phydev = phy_connect(netdev, phydev_name(port_priv->phydev),
> +						adin1110_adjust_link, PHY_INTERFACE_MODE_INTERNAL);
> +		if (IS_ERR(port_priv->phydev)) {
> +			netdev_err(netdev, "Could not connect PHY with device address: %d.\n", i);
> +			return PTR_ERR(port_priv->phydev);
> +		}
> +
> +		ret = devm_add_action_or_reset(dev, adin1110_disconnect_phy, port_priv->phydev);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/* ADIN1110 INT_N pin will be used to signal the host */
> +	ret = devm_request_threaded_irq(dev, priv->spidev->irq, NULL, adin1110_irq,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT, dev_name(dev), priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = register_netdevice_notifier(&adin1110_netdevice_nb);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = register_switchdev_blocking_notifier(&adin1110_switchdev_blocking_notifier);
> +	if (ret < 0) {
> +		unregister_netdevice_notifier(&adin1110_netdevice_nb);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(dev, adin1110_unregister_notifiers, NULL);
> +}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ