Re: sys/cnmac: fix command buffer leak on transmit failure

Visa Hankala <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
On Thu, Jul 09, 2026 at 12:28:47AM +0200, Kirill A. Korinsky wrote:
> visa@,
> 
> my ER-4 whcih I used as router had reached interesting state: it floods
> errors like this:
> 
> Jun 30 11:11:39 gw /bsd: cnmac2: cannot allocate command buffer from free pool allocator
> Jun 30 11:11:39 gw /bsd: cnmac2: failed to transmit packet
> Jun 30 11:11:40 gw /bsd: cnmac2: cannot allocate command buffer from free pool allocator
> Jun 30 11:11:40 gw /bsd: cnmac2: failed to transmit packet
> 
> tons of them, I have no idea what had happened before I discovered it in
> that state, but device was ok via serial, but network was dead.
> 
> After reading code near that error I think I had spotted a leak.

It seems to me that there is no leak in the code. The local cmdptr
pointer is derived using cmdptr_idx and discarded when the function
returns. cnmac_send_cmd() updates cmdptr_idx only if it was able to
add the command words to the queue.

> Index: sys/arch/octeon/dev/if_cnmac.c
> ===================================================================
> RCS file: /home/cvs/src/sys/arch/octeon/dev/if_cnmac.c,v
> diff -u -p -r1.91 if_cnmac.c
> --- sys/arch/octeon/dev/if_cnmac.c	19 Jun 2026 15:12:10 -0000	1.91
> +++ sys/arch/octeon/dev/if_cnmac.c	8 Jul 2026 22:20:34 -0000
> @@ -918,9 +918,6 @@ cnmac_send_cmd(struct cnmac_softc *sc, u
>  
>  	OCTEON_ETH_KASSERT(cmdptr != NULL);
>  
> -	*cmdptr++ = pko_cmd_w0;
> -	*cmdptr++ = pko_cmd_w1;
> -
>  	OCTEON_ETH_KASSERT(sc->sc_cmdptr.cmdptr_idx + 2 <= FPA_COMMAND_BUFFER_POOL_NWORDS - 1);
>  
>  	if (sc->sc_cmdptr.cmdptr_idx + 2 == FPA_COMMAND_BUFFER_POOL_NWORDS - 1) {
> @@ -934,10 +931,14 @@ cnmac_send_cmd(struct cnmac_softc *sc, u
>  			result = 1;
>  			goto done;
>  		}
> +		*cmdptr++ = pko_cmd_w0;
> +		*cmdptr++ = pko_cmd_w1;
>  		*cmdptr++ = buf;
>  		sc->sc_cmdptr.cmdptr = (uint64_t)buf;
>  		sc->sc_cmdptr.cmdptr_idx = 0;
>  	} else {
> +		*cmdptr++ = pko_cmd_w0;
> +		*cmdptr++ = pko_cmd_w1;
>  		sc->sc_cmdptr.cmdptr_idx += 2;
>  	}
>  
> 
> 
> -- 
> wbr, Kirill
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.