Re: [ECOS] bug in eth_drv_send (memory overwrite)

[Gary Thomas <gary at mlbassoc dot com>]

On Wed, 2006-01-18 at 13:18 +0100, Deak, Ferenc wrote:
> Hi!
> 
> I think I found a problem in io/eth/current/src/net/eth_drv.c in eth_drv_send
> I use the current CVS.
> 
> In eth_drv.c at line 706 there is a check 
> "if ( MAX_ETH_DRV_SG < sg_len )"
> I think it have to be
> "if ( MAX_ETH_DRV_SG <= sg_len )"
> 
> for explanation see the snippet at the end of teh file from eth_drv.c, look for lines containing !!!!!
> 
> By the way is there any better solution for the problem (MAX_ETH_DRV_SG <= sg_len) 
> than simply "drop it on the floor" (comment from the file). It's not too nice!
> 

No, if you change the '<' to '<=', you'd stop one short of using
all of the list.

As for what to do, if I had good ideas, they'd already be in there.
The chances of this are extremely remote (never been seen in the wild!)
and to repair all one would need to do is increase the size of the list.
It's kept small just to save space, but in reality it only takes up
8 bytes per slot, so an increase would be minimal.  Of course, the
default size is tunable using CDL.

> 
> 
>         sg_len = 0;  total_len = 0;
>         for (m = m0; m ; m = m->m_next) {
>             data = mtod(m, u_char *);
>             len = m->m_len;
>             total_len += len;
>             sg_list[sg_len].buf = (CYG_ADDRESS)data;
>             sg_list[sg_len].len = len;	// !!!!!! largest sg_len here is MAX_ETH_DRV_SG
>             if ( len )
>                 sg_len++;			// !!!!!! largest sg_len here is MAX_ETH_DRV_SG+1 (after ++)
> #ifdef CYGDBG_IO_ETH_DRIVERS_DEBUG
>             if (cyg_io_eth_net_debug) {
>                 START_CONSOLE();
>                 diag_printf("xmit %d bytes at %p sg[%d]\n", len, data, sg_len);
>                 if ( cyg_io_eth_net_debug > 1)
>                     diag_dump_buf(data, len);
>                 END_CONSOLE();
>             }
> #endif
>             if ( MAX_ETH_DRV_SG < sg_len ) { // !!!!!! true at sg_len = MAX_ETH_DRV_SG+1
> #ifdef CYGPKG_IO_ETH_DRIVERS_WARN_NO_MBUFS
>                 int needed = 0;
>                 struct mbuf *m1;
>                 for (m1 = m0; m1 ; m1 = m1->m_next) needed++;
>                 START_CONSOLE();
>                 diag_printf("too many mbufs to tx, %d > %d, need %d\n", 
>                             sg_len, MAX_ETH_DRV_SG, needed );
>                 END_CONSOLE();
> #endif
>                 sg_len = 0;
>                 break; // drop it on the floor
>             }
>         }
> 
> Best regards,
> Ferenc Deak
> 

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss