netdev - Re: [PATCH net-next 1/5] net: ipqess: introduce the Qualcomm IPQESS 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: <YmMN37VjQNwhLDuX@lunn.ch>
Date:   Fri, 22 Apr 2022 22:19:43 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc:     davem@...emloft.net, Rob Herring <robh+dt@...nel.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, thomas.petazzoni@...tlin.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        linux-arm-kernel@...ts.infradead.org,
        Vladimir Oltean <vladimir.oltean@....com>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Robert Marko <robert.marko@...tura.hr>
Subject: Re: [PATCH net-next 1/5] net: ipqess: introduce the Qualcomm IPQESS
 driver

> +static int ipqess_axi_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct net_device *netdev;
> +	phy_interface_t phy_mode;
> +	struct resource *res;
> +	struct ipqess *ess;
> +	int i, err = 0;
> +
> +	netdev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(struct ipqess),
> +					 IPQESS_NETDEV_QUEUES,
> +					 IPQESS_NETDEV_QUEUES);
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	ess = netdev_priv(netdev);
> +	ess->netdev = netdev;
> +	ess->pdev = pdev;
> +	spin_lock_init(&ess->stats_lock);
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
> +	platform_set_drvdata(pdev, netdev);

....

> +
> +	ipqess_set_ethtool_ops(netdev);
> +
> +	err = register_netdev(netdev);
> +	if (err)
> +		goto err_out;

Before register_netdev() even returns, your devices can be in use, the
open callback called and packets sent. This is particularly true for
NFS root. Which means any setup done after this is probably wrong.

> +
> +	err = ipqess_hw_init(ess);
> +	if (err)
> +		goto err_out;
> +
> +	for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) {
> +		int qid;
> +
> +		netif_tx_napi_add(netdev, &ess->tx_ring[i].napi_tx,
> +				  ipqess_tx_napi, 64);
> +		netif_napi_add(netdev,
> +			       &ess->rx_ring[i].napi_rx,
> +			       ipqess_rx_napi, 64);
> +
> +		qid = ess->tx_ring[i].idx;
> +		err = devm_request_irq(&ess->netdev->dev, ess->tx_irq[qid],
> +				       ipqess_interrupt_tx, 0,
> +				       ess->tx_irq_names[qid],
> +				       &ess->tx_ring[i]);
> +		if (err)
> +			goto err_out;
> +
> +		qid = ess->rx_ring[i].idx;
> +		err = devm_request_irq(&ess->netdev->dev, ess->rx_irq[qid],
> +				       ipqess_interrupt_rx, 0,
> +				       ess->rx_irq_names[qid],
> +				       &ess->rx_ring[i]);
> +		if (err)
> +			goto err_out;
> +	}

All this should probably go before netdev_register().

> +static int ipqess_get_strset_count(struct net_device *netdev, int sset)
> +{
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		return ARRAY_SIZE(ipqess_stats);
> +	default:
> +		netdev_dbg(netdev, "%s: Invalid string set", __func__);

Unsupported would be better than invalid.

> +		return -EOPNOTSUPP;
> +	}
> +}

  Andrew