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]
Date:   Tue, 19 Jul 2022 13:31:11 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Aleksander Jan Bajkowski <olek2@...pl>, hauke@...ke-m.de,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 2/2] net: lantiq_xrx200: fix return value in ENOMEM
 case

On Sun, 2022-07-17 at 21:48 +0200, Aleksander Jan Bajkowski wrote:
> The xrx200_hw_receive() function can return:
> * XRX200_DMA_PACKET_IN_PROGRESS (the next descriptor contains the
> further part of the packet),
> * XRX200_DMA_PACKET_COMPLETE (a complete packet has been received),
> * -ENOMEM (a memory allocation error occurred).
> 
> Currently, the third of these cases is incorrectly handled. The NAPI
> poll function returns then a negative value (-ENOMEM). Correctly in
> such a situation, the driver should try to receive next packet in
> the hope that this time the memory allocation for the next descriptor
> will succeed.

> This patch replaces the XRX200_DMA_PACKET_IN_PROGRESS definition with
> -EINPROGRESS to simplify the driver.
> 
> Fixes: c3e6b2c35b34 ("net: lantiq_xrx200: add ingress SG DMA support")
> Signed-off-by: Aleksander Jan Bajkowski <olek2@...pl>
> ---
>  drivers/net/ethernet/lantiq_xrx200.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
> index 6a83a6c19484..2865d07f3fc8 100644
> --- a/drivers/net/ethernet/lantiq_xrx200.c
> +++ b/drivers/net/ethernet/lantiq_xrx200.c
> @@ -27,9 +27,6 @@
>  #define XRX200_DMA_TX		1
>  #define XRX200_DMA_BURST_LEN	8
>  
> -#define XRX200_DMA_PACKET_COMPLETE	0
> -#define XRX200_DMA_PACKET_IN_PROGRESS	1
> -
>  /* cpu port mac */
>  #define PMAC_RX_IPG		0x0024
>  #define PMAC_RX_IPG_MASK	0xf
> @@ -272,9 +269,8 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
>  		netif_receive_skb(ch->skb_head);
>  		ch->skb_head = NULL;
>  		ch->skb_tail = NULL;
> -		ret = XRX200_DMA_PACKET_COMPLETE;
>  	} else {
> -		ret = XRX200_DMA_PACKET_IN_PROGRESS;
> +		ret = -EINPROGRESS;
>  	}
>  
>  	return ret;
> @@ -292,10 +288,8 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
>  
>  		if ((desc->ctl & (LTQ_DMA_OWN | LTQ_DMA_C)) == LTQ_DMA_C) {
>  			ret = xrx200_hw_receive(ch);
> -			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
> +			if (ret)
>  				continue;
> -			if (ret != XRX200_DMA_PACKET_COMPLETE)
> -				return ret;

Note that in case of persistent pressure and with the device under
flood, 'rx' is not incremented, and this loop can run for an unbound
number of iteration.

If you keep running, you do need to increment 'rx' at every iteration,
even in case of error.

Note that 'rx' is more an estimate of the work done than a 'received
packet counter'.

Paolo

Powered by blists - more mailing lists