[PATCH net v4] net: axienet: safely drop oversized RX frames

Can Ayberk Demir posted 1 patch 7 months ago
.../net/ethernet/xilinx/xilinx_axienet_main.c | 47 +++++++++++--------
1 file changed, 28 insertions(+), 19 deletions(-)
[PATCH net v4] net: axienet: safely drop oversized RX frames
Posted by Can Ayberk Demir 7 months ago
From: Can Ayberk DEMIR <ayberkdemir@gmail.com>

In AXI Ethernet (axienet) driver, receiving an Ethernet frame larger
than the allocated skb buffer may cause memory corruption or kernel panic,
especially when the interface MTU is small and a jumbo frame is received.

This bug was discovered during testing on a Kria K26 platform. When an
oversized frame is received and `skb_put()` is called without checking
the tailroom, the following kernel panic occurs:

  skb_panic+0x58/0x5c
  skb_put+0x90/0xb0
  axienet_rx_poll+0x130/0x4ec
  ...
  Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")

Signed-off-by: Can Ayberk DEMIR <ayberkdemir@gmail.com>
Tested-by: Suraj Gupta <suraj.gupta2@amd.com>
---
Changes in v4:
- Moved Fixes: tag before SOB as requested
- Added Tested-by tag from Suraj Gupta

Changes in v3:
- Fixed 'ndev' undeclared error → replaced with 'lp->ndev'
- Added rx_dropped++ for statistics
- Added Fixes: tag

Changes in v2:
- This patch addresses style issues pointed out in v1.
---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 47 +++++++++++--------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 1b7a653c1f4e..7a12132e2b7c 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1223,28 +1223,37 @@ static int axienet_rx_poll(struct napi_struct *napi, int budget)
 			dma_unmap_single(lp->dev, phys, lp->max_frm_size,
 					 DMA_FROM_DEVICE);
 
-			skb_put(skb, length);
-			skb->protocol = eth_type_trans(skb, lp->ndev);
-			/*skb_checksum_none_assert(skb);*/
-			skb->ip_summed = CHECKSUM_NONE;
-
-			/* if we're doing Rx csum offload, set it up */
-			if (lp->features & XAE_FEATURE_FULL_RX_CSUM) {
-				csumstatus = (cur_p->app2 &
-					      XAE_FULL_CSUM_STATUS_MASK) >> 3;
-				if (csumstatus == XAE_IP_TCP_CSUM_VALIDATED ||
-				    csumstatus == XAE_IP_UDP_CSUM_VALIDATED) {
-					skb->ip_summed = CHECKSUM_UNNECESSARY;
+			if (unlikely(length > skb_tailroom(skb))) {
+				netdev_warn(lp->ndev,
+					    "Dropping oversized RX frame (len=%u, tailroom=%u)\n",
+					    length, skb_tailroom(skb));
+				dev_kfree_skb(skb);
+				lp->ndev->stats.rx_dropped++;
+				skb = NULL;
+			} else {
+				skb_put(skb, length);
+				skb->protocol = eth_type_trans(skb, lp->ndev);
+				/*skb_checksum_none_assert(skb);*/
+				skb->ip_summed = CHECKSUM_NONE;
+
+				/* if we're doing Rx csum offload, set it up */
+				if (lp->features & XAE_FEATURE_FULL_RX_CSUM) {
+					csumstatus = (cur_p->app2 &
+							XAE_FULL_CSUM_STATUS_MASK) >> 3;
+					if (csumstatus == XAE_IP_TCP_CSUM_VALIDATED ||
+					    csumstatus == XAE_IP_UDP_CSUM_VALIDATED) {
+						skb->ip_summed = CHECKSUM_UNNECESSARY;
+					}
+				} else if (lp->features & XAE_FEATURE_PARTIAL_RX_CSUM) {
+					skb->csum = be32_to_cpu(cur_p->app3 & 0xFFFF);
+					skb->ip_summed = CHECKSUM_COMPLETE;
 				}
-			} else if (lp->features & XAE_FEATURE_PARTIAL_RX_CSUM) {
-				skb->csum = be32_to_cpu(cur_p->app3 & 0xFFFF);
-				skb->ip_summed = CHECKSUM_COMPLETE;
-			}
 
-			napi_gro_receive(napi, skb);
+				napi_gro_receive(napi, skb);
 
-			size += length;
-			packets++;
+				size += length;
+				packets++;
+			}
 		}
 
 		new_skb = napi_alloc_skb(napi, lp->max_frm_size);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH net v4] net: axienet: safely drop oversized RX frames
Posted by Eric Dumazet 7 months ago
On Fri, May 16, 2025 at 1:44 AM Can Ayberk Demir <ayberkdemir@gmail.com> wrote:
>
> From: Can Ayberk DEMIR <ayberkdemir@gmail.com>
>
> In AXI Ethernet (axienet) driver, receiving an Ethernet frame larger
> than the allocated skb buffer may cause memory corruption or kernel panic,
> especially when the interface MTU is small and a jumbo frame is received.
>
> This bug was discovered during testing on a Kria K26 platform. When an
> oversized frame is received and `skb_put()` is called without checking
> the tailroom, the following kernel panic occurs:
>
>   skb_panic+0x58/0x5c
>   skb_put+0x90/0xb0
>   axienet_rx_poll+0x130/0x4ec
>   ...
>   Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt
>
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
>
> Signed-off-by: Can Ayberk DEMIR <ayberkdemir@gmail.com>
> Tested-by: Suraj Gupta <suraj.gupta2@amd.com>
> ---
> Changes in v4:
> - Moved Fixes: tag before SOB as requested
> - Added Tested-by tag from Suraj Gupta
>
> Changes in v3:
> - Fixed 'ndev' undeclared error → replaced with 'lp->ndev'
> - Added rx_dropped++ for statistics
> - Added Fixes: tag
>
> Changes in v2:
> - This patch addresses style issues pointed out in v1.
> ---
>  .../net/ethernet/xilinx/xilinx_axienet_main.c | 47 +++++++++++--------
>  1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 1b7a653c1f4e..7a12132e2b7c 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -1223,28 +1223,37 @@ static int axienet_rx_poll(struct napi_struct *napi, int budget)
>                         dma_unmap_single(lp->dev, phys, lp->max_frm_size,
>                                          DMA_FROM_DEVICE);
>
> -                       skb_put(skb, length);
> -                       skb->protocol = eth_type_trans(skb, lp->ndev);
> -                       /*skb_checksum_none_assert(skb);*/
> -                       skb->ip_summed = CHECKSUM_NONE;
> -
> -                       /* if we're doing Rx csum offload, set it up */
> -                       if (lp->features & XAE_FEATURE_FULL_RX_CSUM) {
> -                               csumstatus = (cur_p->app2 &
> -                                             XAE_FULL_CSUM_STATUS_MASK) >> 3;
> -                               if (csumstatus == XAE_IP_TCP_CSUM_VALIDATED ||
> -                                   csumstatus == XAE_IP_UDP_CSUM_VALIDATED) {
> -                                       skb->ip_summed = CHECKSUM_UNNECESSARY;
> +                       if (unlikely(length > skb_tailroom(skb))) {

If really the NIC copied more data than allowed, we already have
corruption of kernel memory.

Dropping the packet here has undetermined behavior.

If the NIC only reports the big length but has not performed any DMA,
then the skb can be recycled.
No point freeing it, and re-allocate a new one.