[PATCH net v2] net: bgmac: fix oversized RX frame check

[PATCH net v2] net: bgmac: fix oversized RX frame check

[Rosen Penev]

While reviewing commit 77dac783f11
("net: bgmac: rate limit RX error messages") , Florian spotted that the RX
length sanity check compares the length reported in the RX header against
BGMAC_RX_ALLOC_SIZE. That is the size of the whole fragment, including the
space reserved for struct skb_shared_info. The data area handed to the
hardware is only BGMAC_RX_BUF_SIZE bytes, and the frame starts
BGMAC_RX_FRAME_OFFSET bytes into it, so a valid frame is at most
BGMAC_RX_MAX_FRAME_SIZE bytes long.

A corrupted length between those two limits passes the check, and the
following skb_put() runs past the skb tailroom and calls
skb_over_panic() instead of dropping the frame.

Compare against BGMAC_RX_MAX_FRAME_SIZE instead.

Only compile tested. Not tested on hardware.

Fixes: 6a6c708469c9 ("bgmac: add check for oversized packets")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: replace Suggested-by with description.
 drivers/net/ethernet/broadcom/bgmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index b3a92ae95de3..6873c7bb7a14 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -455,7 +455,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
 				break;
 			}
 
-			if (len > BGMAC_RX_ALLOC_SIZE) {
+			if (len > BGMAC_RX_MAX_FRAME_SIZE) {
 				if (net_ratelimit())
 					netdev_err(bgmac->net_dev, "Found oversized packet at slot %d, DMA issue!\n",
 						   ring->start);
-- 
2.56.0

Re: [PATCH net v2] net: bgmac: fix oversized RX frame check

[Chenguang Zhao]

On Tue, Oct 27, 2026 at 06:10:35PM -0700, Rosen Penev wrote:
> While reviewing commit 77dac783f11
> ("net: bgmac: rate limit RX error messages") , Florian spotted that the RX
> length sanity check compares the length reported in the RX header against
> BGMAC_RX_ALLOC_SIZE. That is the size of the whole fragment, including the
> space reserved for struct skb_shared_info. The data area handed to the
> hardware is only BGMAC_RX_BUF_SIZE bytes, and the frame starts
> BGMAC_RX_FRAME_OFFSET bytes into it, so a valid frame is at most
> BGMAC_RX_MAX_FRAME_SIZE bytes long.
> 
> A corrupted length between those two limits passes the check, and the
> following skb_put() runs past the skb tailroom and calls
> skb_over_panic() instead of dropping the frame.
> 
> Compare against BGMAC_RX_MAX_FRAME_SIZE instead.
> 
> Only compile tested. Not tested on hardware.
> 
> Fixes: 6a6c708469c9 ("bgmac: add check for oversized packets")
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v2: replace Suggested-by with description.
>  drivers/net/ethernet/broadcom/bgmac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index b3a92ae95de3..6873c7bb7a14 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -455,7 +455,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
>  				break;
>  			}
>  
> -			if (len > BGMAC_RX_ALLOC_SIZE) {
> +			if (len > BGMAC_RX_MAX_FRAME_SIZE) {
>  				if (net_ratelimit())
>  					netdev_err(bgmac->net_dev, "Found oversized packet at slot %d, DMA issue!\n",
>  						   ring->start);
> -- 
> 2.56.0
> 

Hi Rosen

FWIW, it’s helpful for reviewers to have a link to the
prior patch with the new revision for easy reference. :)

Thanks
Chenguang

Re: [PATCH net v2] net: bgmac: fix oversized RX frame check

[Rosen Penev]

On Tue, Oct 27, 2026 at 7:21 PM Chenguang Zhao <chenguang.zhao@linux.dev> wrote:
>
> On Tue, Oct 27, 2026 at 06:10:35PM -0700, Rosen Penev wrote:
> > While reviewing commit 77dac783f11
> > ("net: bgmac: rate limit RX error messages") , Florian spotted that the RX
> > length sanity check compares the length reported in the RX header against
> > BGMAC_RX_ALLOC_SIZE. That is the size of the whole fragment, including the
> > space reserved for struct skb_shared_info. The data area handed to the
> > hardware is only BGMAC_RX_BUF_SIZE bytes, and the frame starts
> > BGMAC_RX_FRAME_OFFSET bytes into it, so a valid frame is at most
> > BGMAC_RX_MAX_FRAME_SIZE bytes long.
> >
> > A corrupted length between those two limits passes the check, and the
> > following skb_put() runs past the skb tailroom and calls
> > skb_over_panic() instead of dropping the frame.
> >
> > Compare against BGMAC_RX_MAX_FRAME_SIZE instead.
> >
> > Only compile tested. Not tested on hardware.
> >
> > Fixes: 6a6c708469c9 ("bgmac: add check for oversized packets")
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v2: replace Suggested-by with description.
> >  drivers/net/ethernet/broadcom/bgmac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> > index b3a92ae95de3..6873c7bb7a14 100644
> > --- a/drivers/net/ethernet/broadcom/bgmac.c
> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
> > @@ -455,7 +455,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
> >                               break;
> >                       }
> >
> > -                     if (len > BGMAC_RX_ALLOC_SIZE) {
> > +                     if (len > BGMAC_RX_MAX_FRAME_SIZE) {
> >                               if (net_ratelimit())
> >                                       netdev_err(bgmac->net_dev, "Found oversized packet at slot %d, DMA issue!\n",
> >                                                  ring->start);
> > --
> > 2.56.0
> >
>
> Hi Rosen
>
> FWIW, it’s helpful for reviewers to have a link to the
> prior patch with the new revision for easy reference. :)
https://lore.kernel.org/netdev/20261023005313.3946390-1-rosenp@gmail.com/
>
> Thanks
> Chenguang