[PATCH net-next] net: stmmac: fix rx limit check in stmmac_rx_zc()

[PATCH net-next] net: stmmac: fix rx limit check in stmmac_rx_zc()

[Alexey Kodanev]

The extra "count >= limit" check in stmmac_rx_zc() is redundant and
has no effect because the value of "count" doesn't change after the
while condition at this point.

However, it can change after "read_again:" label:

        while (count < limit) {
            ...

            if (count >= limit)
                break;
    read_again:
            ...
            /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
            if (likely(status & rx_not_ls)) {
                xsk_buff_free(buf->xdp);
                buf->xdp = NULL;
                dirty++;
                count++;
                goto read_again;
            }
            ...

This patch addresses the same issue previously resolved in stmmac_rx()
by commit fa02de9e7588 ("net: stmmac: fix rx budget limit check").
The fix is the same: move the check after the label to ensure that it
bounds the goto loop.

Detected using the static analysis tool - Svace.
Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
---

After creating the patch, I also found the previous attempt to fix this issue
from 2023, but I'm not sure what went wrong or why it wasn't applied:
https://lore.kernel.org/netdev/ZBRd2HyZdz52eXyz@nimitz/

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7b90ecd3a55e..86e912471dea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5258,10 +5258,10 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
 			len = 0;
 		}
 
+read_again:
 		if (count >= limit)
 			break;
 
-read_again:
 		buf1_len = 0;
 		entry = next_entry;
 		buf = &rx_q->buf_pool[entry];
-- 
2.25.1

Re: [PATCH net-next] net: stmmac: fix rx limit check in stmmac_rx_zc()

[Russell King (Oracle)]

On Wed, Nov 26, 2025 at 10:43:27AM +0000, Alexey Kodanev wrote:
> The extra "count >= limit" check in stmmac_rx_zc() is redundant and
> has no effect because the value of "count" doesn't change after the
> while condition at this point.
> 
> However, it can change after "read_again:" label:
> 
>         while (count < limit) {
>             ...
> 
>             if (count >= limit)
>                 break;
>     read_again:
>             ...
>             /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
>             if (likely(status & rx_not_ls)) {
>                 xsk_buff_free(buf->xdp);
>                 buf->xdp = NULL;
>                 dirty++;
>                 count++;
>                 goto read_again;
>             }
>             ...
> 
> This patch addresses the same issue previously resolved in stmmac_rx()
> by commit fa02de9e7588 ("net: stmmac: fix rx budget limit check").
> The fix is the same: move the check after the label to ensure that it
> bounds the goto loop.
> 
> Detected using the static analysis tool - Svace.
> Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
> Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
> ---
> 
> After creating the patch, I also found the previous attempt to fix this issue
> from 2023, but I'm not sure what went wrong or why it wasn't applied:
> https://lore.kernel.org/netdev/ZBRd2HyZdz52eXyz@nimitz/

It was because:

https://lore.kernel.org/netdev/871qli0wap.fsf@henneberg-systemdesign.com/

indicated that the author was going to do further work on the patchset,
so the patch submission was marked as "Changes Requested":

https://patchwork.kernel.org/project/netdevbpf/list/?series=730639&state=*

My guess is the author never came back with any patches.

netdev is based on patchwork, which means once a patch series has been
marked in such a way that it isn't going to be applied, it won't get
looked at again, and it's up to the author to resubmit. If the author
doesn't resubmit, no action will happens, especially for a driver such
as stmmac which doesn't have a maintainer.

I think this is a safe change.

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next] net: stmmac: fix rx limit check in stmmac_rx_zc()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Nov 2025 10:43:27 +0000 you wrote:
> The extra "count >= limit" check in stmmac_rx_zc() is redundant and
> has no effect because the value of "count" doesn't change after the
> while condition at this point.
> 
> However, it can change after "read_again:" label:
> 
>         while (count < limit) {
>             ...
> 
> [...]

Here is the summary with links:
  - [net-next] net: stmmac: fix rx limit check in stmmac_rx_zc()
    https://git.kernel.org/netdev/net-next/c/8048168df56e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html