[PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure

[PATCH net v2 0/3] net: lantiq_xrx200: fix errors under memory pressure

[Aleksander Jan Bajkowski]

This series fixes issues that can occur in the driver under memory pressure.
Situations when the system cannot allocate memory are rare, so the mentioned bugs
have been fixed recently. The patches have been tested on a BT Home router with the
Lantiq xRX200 chipset.

Changelog:

  v2:
  - the second patch has been changed, so that under memory pressure situation
    the driver will not receive packets indefinitely regardless of the NAPI budget,
  - the third patch has been added.

Aleksander Jan Bajkowski (3):
  net: lantiq_xrx200: confirm skb is allocated before using
  net: lantiq_xrx200: fix lock under memory pressure
  net: lantiq_xrx200: restore buffer if memory allocation failed

 drivers/net/ethernet/lantiq_xrx200.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.30.2

[PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using

[Aleksander Jan Bajkowski]

xrx200_hw_receive() assumes build_skb() always works and goes straight
to skb_reserve(). However, build_skb() can fail under memory pressure.

Add a check in case build_skb() failed to allocate and return NULL.

Fixes: e015593573b3 ("net: lantiq_xrx200: convert to build_skb")
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 5edb68a8aab1..6a83a6c19484 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -239,6 +239,13 @@ static int xrx200_hw_receive(struct xrx200_chan *ch)
 	}
 
 	skb = build_skb(buf, priv->rx_skb_size);
+	if (!skb) {
+		skb_free_frag(buf);
+		net_dev->stats.rx_dropped++;
+		netdev_err(net_dev, "failed to build skb\n");
+		return -ENOMEM;
+	}
+
 	skb_reserve(skb, NET_SKB_PAD);
 	skb_put(skb, len);
 
-- 
2.30.2

Re: [PATCH net v2 1/3] net: lantiq_xrx200: confirm skb is allocated before using

[Jakub Kicinski]

On Mon, 15 Aug 2022 16:57:38 +0200 Aleksander Jan Bajkowski wrote:
> +		netdev_err(net_dev, "failed to build skb\n");

I don't thinkg this is needed, is build_skb() using GFP_NOWARN?
Otherwise there will be an OOM splat anyway. Do check, I'm not sure.

If there won't be an OOM splat this line should be rate limited.

[PATCH net v2 2/3] net: lantiq_xrx200: fix lock under memory pressure

[Aleksander Jan Bajkowski]

When the xrx200_hw_receive() function returns -ENOMEM, the NAPI poll
function immediately returns an error.
This is incorrect for two reasons:
* the function terminates without enabling interrupts or scheduling NAPI,
* the error code (-ENOMEM) is returned instead of the number of received
packets.

After the first memory allocation failure occurs, packet reception is
locked due to disabled interrupts from DMA..

Fixes: fe1a56420cf2 ("net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 6a83a6c19484..8f9155eacdb3 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -295,7 +295,7 @@ static int xrx200_poll_rx(struct napi_struct *napi, int budget)
 			if (ret == XRX200_DMA_PACKET_IN_PROGRESS)
 				continue;
 			if (ret != XRX200_DMA_PACKET_COMPLETE)
-				return ret;
+				break;
 			rx++;
 		} else {
 			break;
-- 
2.30.2

[PATCH net v2 3/3] net: lantiq_xrx200: restore buffer if memory allocation failed

[Aleksander Jan Bajkowski]

In a situation where memory allocation fails, an invalid buffer address
is stored. When this descriptor is used again, the system panics in the
build_skb() function when accessing memory.

Fixes: 7ea6cd16f159 ("lantiq: net: fix duplicated skb in rx descriptor ring")
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/ethernet/lantiq_xrx200.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/lantiq_xrx200.c b/drivers/net/ethernet/lantiq_xrx200.c
index 8f9155eacdb3..e817bc545d81 100644
--- a/drivers/net/ethernet/lantiq_xrx200.c
+++ b/drivers/net/ethernet/lantiq_xrx200.c
@@ -193,6 +193,7 @@ static int xrx200_alloc_buf(struct xrx200_chan *ch, void *(*alloc)(unsigned int
 
 	ch->rx_buff[ch->dma.desc] = alloc(priv->rx_skb_size);
 	if (!ch->rx_buff[ch->dma.desc]) {
+		ch->rx_buff[ch->dma.desc] = buf;
 		ret = -ENOMEM;
 		goto skip;
 	}
-- 
2.30.2