netdev - Re: [PATCH net-next v9 12/14] xsc: Add ndo_start

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: <20250326101727.GA892515@horms.kernel.org>
Date: Wed, 26 Mar 2025 10:17:27 +0000
From: Simon Horman <horms@...nel.org>
To: Xin Tian <tianx@...silicon.com>
Cc: netdev@...r.kernel.org, leon@...nel.org, andrew+netdev@...n.ch,
	kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
	davem@...emloft.net, jeff.johnson@....qualcomm.com,
	przemyslaw.kitszel@...el.com, weihg@...silicon.com,
	wanry@...silicon.com, jacky@...silicon.com,
	parthiban.veerasooran@...rochip.com, masahiroy@...nel.org,
	kalesh-anakkur.purayil@...adcom.com, geert+renesas@...der.be,
	geert@...ux-m68k.org
Subject: Re: [PATCH net-next v9 12/14] xsc: Add ndo_start_xmit

On Tue, Mar 18, 2025 at 11:15:19PM +0800, Xin Tian wrote:

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_tx.c b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_tx.c

...

> +netdev_tx_t xsc_eth_xmit_start(struct sk_buff *skb, struct net_device *netdev)
> +{
> +	struct xsc_adapter *adapter = netdev_priv(netdev);
> +	int ds_num = adapter->xdev->caps.send_ds_num;

Hi Xin Tian,

adapter and adapter->xdev are dereferenced here...

> +	struct xsc_tx_wqe *wqe;
> +	struct xsc_sq *sq;
> +	u16 pi;
> +
> +	if (!adapter ||
> +	    !adapter->xdev ||

... but it is assumed here that both adapter and adapter->xdev may be NULL.

This seems inconsistent.

I haven't looked but I do wonder if adapter or adapter-xdev can be NULL in
practice?. If not, the checks above can be dropped. If so, then ds_num
should be assigned after these checks.

Flagged by Smatch.

> +	    adapter->status != XSCALE_ETH_DRIVER_OK)
> +		return NETDEV_TX_BUSY;
> +
> +	sq = adapter->txq2sq[skb_get_queue_mapping(skb)];
> +	if (unlikely(!sq))
> +		return NETDEV_TX_BUSY;
> +
> +	wqe = xsc_sq_fetch_wqe(sq, ds_num * XSC_SEND_WQE_DS, &pi);
> +
> +	return xsc_eth_xmit_frame(skb, sq, wqe, pi);
> +}

...