buglwIP - A Lightweight TCP/IP stack - Bugs: bug #60532, Double free and assert...

 
 

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

bug #60532: Double free and assert "pbuf_free: p->ref > 0" fail

Submitter:  SviMik <svimik>
Submitted:  Wed 05 May 2021 01:48:59 PM UTC
   
 
Category:  pbufs Severity:  3 - Normal
Item Group:  Crash Error Status:  Invalid
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  Other

Jump to the original submission

Sat 08 May 2021 11:13:18 PM UTC, comment #6: 

Thanks for your help! Maybe some LWIP_ASSERT could be put there to ensure this condition is met?

SviMik <svimik>
Thu 06 May 2021 10:16:19 AM UTC, comment #5: 

See also MEM_LIBC_MALLOC if you want to use standard heap functions.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 06 May 2021 10:14:15 AM UTC, comment #4: 

Implementing mem_trim() using realloc() is not supported. See the documentation of mem_trim():

"@return for compatibility reasons: is always == rmem, at the moment or NULL if newsize is > old size, in which case rmem is NOT touched or freed!"

-> closing as invalid

Simon Goldschmidt <goldsimon>
Group administrator
Wed 05 May 2021 11:12:29 PM UTC, comment #3: 

The website has converted some asterisks to bold tags, but I hope you get the idea. It's, of course, void * mem_trim (returns a pointer).

SviMik <svimik>
Wed 05 May 2021 11:07:19 PM UTC, comment #2: 

The problem happens when mem_trim() is called in pbuf.c

In my project mem_trim() is implemented via realloc() in the following way:

void *mem_trim(void *mem, mem_size_t size){
return realloc(mem, size);
}

The problem is that realloc() returns a new pointer that must be saved, the old pointer is considered free'd (that's exactly what valgrind says) and can't be used anymore. So what happens is that lwip keeps using the old pointer ignoring the return value of mem_trim().

I'm reading the code in pbuf.c but I can't see how the new pointer is stored:

    /* reallocate and adjust the length of the pbuf that will be split */
    q = (struct pbuf )mem_trim(q, (mem_size_t)(((u8_t )q->payload - (u8_t *)q) + rem_len));
    LWIP_ASSERT("mem_trim returned q == NULL", q != NULL);
  }
  /* adjust length fields for new last pbuf */
  q->len = rem_len;
  q->tot_len = q->len;

  /* any remaining pbufs in chain? */
  if (q->next != NULL) {
    /* free remaining pbufs in chain */
    pbuf_free(q->next);
  }
  /* q is last packet in chain */
  q->next = NULL;

} <--- at this point the q will be lost forever

SviMik <svimik>
Wed 05 May 2021 05:57:49 PM UTC, comment #1: 
SviMik <svimik>
Wed 05 May 2021 01:48:59 PM UTC, original submission:  

Hi,
I'm experiencing a lot of memory issues in lwip when checking my project with valgrind. One of them seems fatal because lwip assertion fails too.

I'm using git "STABLE-2_1_x" branch.

Here is valgrind log. I'm not sure if the assertion fail is somehow connected with invalid free() or it's two separate issues, there are just too many of them and I really need some help.

==8111== Invalid free() / delete / delete[] / realloc()
==8111==    at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8111==    by 0x11C7FE: mem_free (main.h:1479)
==8111==    by 0x11C7FE: pbuf_free (pbuf.c:780)
==8111==    by 0x121546: tcp_seg_free (tcp.c:1663)
==8111==    by 0x121656: tcp_free_acked_segments(tcp_pcb*, tcp_seg*, char const*, tcp_seg*) [clone .isra.58] (tcp_in.c:1132)
==8111==    by 0x123B84: tcp_receive(tcp_pcb*) (tcp_in.c:1301)
==8111==    by 0x12AAB1: tcp_process (tcp_in.c:999)
==8111==    by 0x12AAB1: tcp_input (tcp_in.c:454)
==8111==    by 0x12B645: ip4_input (ip4.c:717)
==8111==    by 0x12096E: lwip_push_ip_packet(session*, void const*, int) (tap.c:1166)
==8111==    by 0x132C8F: process_incoming_tap_packet(session*, unsigned char*, int) (tap.c:1968)
==8111==    by 0x132E52: tap_process_thread(void*) (tap_linux.c:647)
==8111==    by 0x654C6DA: start_thread (pthread_create.c:463)
==8111==    by 0x688571E: clone (clone.S:95)
==8111==  Address 0x1029b0f0 is 0 bytes inside a block of size 1,520 free'd
==8111==    at 0x4C33D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8111==    by 0x11D38A: mem_trim (main.h:1470)
==8111==    by 0x11D38A: pbuf_realloc (pbuf.c:444)
==8111==    by 0x122E65: tcp_split_unsent_seg (tcp_out.c:932)
==8111==    by 0x1290E4: tcp_slowtmr (tcp.c:1290)
==8111==    by 0x12C118: tcpip_tcp_timer(void*) (timeouts.c:149)
==8111==    by 0x12C6C4: sys_check_timeouts (timeouts.c:390)
==8111==    by 0x12C703: lwip_timer(session*) (tap.c:1182)
==8111==    by 0x12CA48: tap_thread(void*) (tap_linux.c:832)
==8111==    by 0x654C6DA: start_thread (pthread_create.c:463)
==8111==    by 0x688571E: clone (clone.S:95)
==8111==  Block was alloc'd at
==8111==    at 0x4C31B0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8111==    by 0x11CEEC: mem_malloc (main.h:1473)
==8111==    by 0x11CEEC: pbuf_alloc (pbuf.c:284)
==8111==    by 0x11D0B2: tcp_pbuf_prealloc(pbuf_layer, unsigned short, unsigned short, unsigned short*, tcp_pcb const*, unsigned char, unsigned char) (tcp_out.c:263)
==8111==    by 0x122579: tcp_write (tcp_out.c:610)
==8111==    by 0x124EA7: lwip_tcp_write(session*, tcp_pcb*, void const*, int) (tap.c:894)
==8111==    by 0x125009: lwip_stream_process_app_buf(_stream*) (tap.c:705)
==8111==    by 0x125C06: client_thread_lwip(void*) (tap.c:757)
==8111==    by 0x654C6DA: start_thread (pthread_create.c:463)
==8111==    by 0x688571E: clone (clone.S:95)
==8111==
[02:40:29] Fatal error in lwip module: pbuf_free: p->ref > 0 in file lwip/pbuf.c in function pbuf_free()

What I tried to do:

1. I'm aware that the raw API isn't thread-safe, so I have wrapped all API calls with my own mutex making sure I'm not calling lwip from different threads simultaneously.

2. I have also enabled SYS_LIGHTWEIGHT_PROT and used sys_arch_protect()/sys_arch_unprotect() implementation from here: https://github.com/dreamcat4/lwip/blob/master/contrib/ports/unix/sys_arch.c

3. Temporarily added printf to sys_arch_protect() to make sure it's actually being called.

4. Found a similar issue here: http://lwip.100.n7.nabble.com/bug-57377-Assertion-quot-pbuf-free-p-gt-ref-gt-0-quot-failed-td35092.html and applied this patch: http://git.savannah.nongnu.org/cgit/lwip.git/commit/?id=e80d4ff2cc5f8f864e9e996c72b47ebefd2a5175 which didn't help.

I'm totally aware that it might be my mistake, but I'm really stuck and don't know what to do.

I have attached the full valgrind log which contains other errors too. I'm not sure if they all are problems or some of them are false positives (valgrind is notorious for its verbosity and false positives), but the "pbuf_free: p->ref > 0" is fatal, and valgrind might be a key to that.

SviMik <svimik>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #51383:  valgrind_1.log added by svimik (243KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-05-06 goldsimon StatusNone Invalid
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2021-05-05 svimik Attached File- Added valgrind_1.log, #51383

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code