This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: bug in eth_drv_send (memory overwrite)
> 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.
I have looked it over again, but in my opinion sg_list[MAX_ETH_DRV_SG] *IS WRITTEN*, and
it is declared as "struct eth_drv_sg sg_list[MAX_ETH_DRV_SG];"
You can't write "array[N]", if it is declared as "sometype array[N]".
> 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.
Yes I know, and we have tuned it already.
We have reached the limit of this list (MAX_ETH_DRV_SG). It is possibly a desing error,
but this is a valid code, calling a lot of write on a socket with len<50. We will redesign
it, but now increasing MAX_ETH_DRV_SG is the quick solution. Perhaps the diag_printf
or a comment here could be more explanatory?
>
> >
> >
> > 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