linux-kernel - Re: [PATCH net] net: mv643xx_eth: implement descriptor cleanup in txq_submit

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] [day] [month] [year] [list]

Message-ID: <32e872e1-e842-4839-beeb-a1e9f235ed42@redhat.com>
Date: Tue, 28 Jan 2025 11:47:45 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@...il.com>,
 sebastian.hesselbarth@...il.com, netdev@...r.kernel.org
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: mv643xx_eth: implement descriptor cleanup in
 txq_submit_tso

On 1/24/25 7:24 AM, Dheeraj Reddy Jonnalagadda wrote:
> Implement cleanup of used descriptors in the error path of txq_submit_tso.
> 
> Fixes: 3ae8f4e0b98b ("net: mv643xx_eth: Implement software TSO")
> Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@...il.com>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 67a6ff07c83d..8d217f8d451e 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -881,10 +881,20 @@ static int txq_submit_tso(struct tx_queue *txq, struct sk_buff *skb,
>  	txq_enable(txq);
>  	txq->tx_desc_count += desc_count;
>  	return 0;
> +
>  err_release:
> -	/* TODO: Release all used data descriptors; header descriptors must not
> +	/* Release all used data descriptors; header descriptors must not
>  	 * be DMA-unmapped.
>  	 */
> +	for (int i = 0; i < desc_count; i++) {

Please move the 'i' variable definition into the initial definition for
this function.

> +		int desc_index = (txq->tx_curr_desc + i) % txq->tx_ring_size;

AFAICS we reach here when the first `desc_count - 1` descriptor for the
current TSO packet has been filled and filling the `desc_count`th one,
in the `txq->tx_curr_desc` index failed.

Given the above, you should free the previous `desc_count - 1`
descriptors, i.e. txq->tx_curr_desc - i.

> +		struct tx_desc *desc = &txq->tx_desc_area[desc_index];
> +		desc->cmd_sts = 0; /* Reset the descriptor */

It looks like you should still unmap data descriptor as done in
txq_reclaim()

/P