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: <ad1566da-1e2e-4bf4-b741-f2e484f4b490@yunsilicon.com>
Date: Wed, 9 Apr 2025 15:36:05 +0800
From: "Xin Tian" <tianx@...silicon.com>
To: "Simon Horman" <horms@...nel.org>
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 2025/3/26 18:17, Simon Horman wrote:
> 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.

It can't be NULL, I'll drop the checks

Thanks

>> +	    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);
>> +}
> ...