buglwIP - A Lightweight TCP/IP stack - Bugs: bug #34429, In RAW/NO_SYS mode, with...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #34429: In RAW/NO_SYS mode, with LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT set to 1, memory gets corrupted

Submitted by:  Min Xu <maximalmin>
Submitted on:  Thu 29 Sep 2011 02:22:04 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.4.0

Sun 23 Oct 2011 03:40:34 PM UTC, comment #5:

Your proposed fix for 1 looks good, but for 2, I'll calculate lfree in a local variable and restart the loop (back to 'if(mem == lfree)') if mem_free has run in between.

Fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 30 Sep 2011 01:12:36 AM UTC, comment #4:

The code changes worked for me... however, looking at the section 2 again, it appears it's possible other users may have a large MEM section to allocate from so it's possible that they need to enable interrupt.. However, doing so may allow lfree to be modified else where so it'll be necessary to add a local variable to move lfree to its proper value.

However, in my test, leaving section 2 as it were (in original 1.4.0 code) would cause memory corruption

Min Xu <maximalmin>
Fri 30 Sep 2011 01:04:17 AM UTC, comment #3:

My change that seem to work:
1: Adding break; after setting local_mem_free_count to 1 if mem_free_count is non-zero. No need to LOAD-STORE if a single STORE instruction suffices

2: Remove the extraneous LWIP_MEM_ALLOC_UNPROTECT() and LWIP_MEM_ALLOC_PROTECT() when mem == lfree .. We don't want another mem_malloc or mem_free to mess up the lfree

Section 1:
for (ptr = (mem_size_t)((u8_t *)lfree - ram); ptr < MEM_SIZE_ALIGNED - size;
ptr = ((struct mem )(void )&ram[ptr])->next) {
mem = (struct mem )(void )&ram[ptr];
#if LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT
mem_free_count = 0;
LWIP_MEM_ALLOC_UNPROTECT();
/* allow mem_free to run */
LWIP_MEM_ALLOC_PROTECT();
if (mem_free_count != 0) {
local_mem_free_count = 1;
break;
}
mem_free_count = 0;
#endif /* LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT */

Section 2:
if (mem == lfree) {
/* Find next free block after mem and update lowest free pointer */
while (lfree->used && lfree != ram_end) {
// LWIP_MEM_ALLOC_UNPROTECT();
/* prevent high interrupt latency... */
// LWIP_MEM_ALLOC_PROTECT();
lfree = (struct mem )(void )&ram[lfree->next];
}
LWIP_ASSERT("mem_malloc: !lfree->used", ((lfree == ram_end) || (!lfree->used)));
}

Min Xu <maximalmin>
Fri 30 Sep 2011 12:44:06 AM UTC, comment #2:

I think it's probably better that if mem_free() ran (and allowed to plug holes), mem_malloc just break out the for loop that way it can start from beginning

Min Xu <maximalmin>
Thu 29 Sep 2011 08:21:38 PM UTC, comment #1:

From reading the code, you're probably right and mem_free() shouldn't change layout of the heap while mem_malloc() is running.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 29 Sep 2011 02:22:04 AM UTC, original submission:

Wen building an application with LwIP on the TI/DSP where the the Transmit interrupt routine (where EMAC module reports it has sent out packet[s]) calls the pbuf_free function on PBUF_RAM and PBUF_POOL type pbufs, setting the stock LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT flag still results in memory corruption.

Currently work around by renaming the mem_malloc, mem_trim and mem_free functions and then add wrap around functions that calls

LWIP_MEM_ALLOC_DECL_PROTECT();
LWIP_MEM_ALLOC_PROTECT();
and
LWIP_MEM_ALLOC_UNPROTECT();

around the original routine.

If there is someone more familiar with this section of code in race condition and more qualified can work on fixing it.

My current guess is that in mem_alloc, inside the for loop
for (ptr = (mem_size_t)((u8_t *)lfree - ram); ptr < MEM_SIZE_ALIGNED - size;
ptr = ((struct mem )(void )&ram[ptr])->next)

It's possible that the mem_free code that may have run between the LWIP_MEM_ALLOC_UNPROTECT() and LWIP_MEM_ALLOC_PROTECT() has possibly invalidated mem (e.g., plug holes etc)

Min Xu <maximalmin>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by maximalmin (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 23 Oct 2011 03:40:34 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Fri 21 Oct 2011 07:49:24 PM UTCgoldsimonPlanned Release=>1.4.1

    Back to the top


    Powered by Savane 3.1-cleanup1