buglwIP - A Lightweight TCP/IP stack - Bugs: bug #57790, Fragmented UDP packets leads to...

 
 

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

bug #57790: Fragmented UDP packets leads to crash on reassembly

Submitter:  Jarno Malmari <jmalmari>
Submitted:  Tue 11 Feb 2020 04:05:23 PM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Crash Error Status:  Invalid
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Jump to the original submission

Thu 24 Sep 2020 09:50:18 AM UTC, comment #13: 

Mentioning threading requirement is a good point, I will check that in detail. But I think I found at least one issue in my source:

In my interface  low level input method:

struct pbuf* p = pbuf_alloc(PBUF_RAW, Msg->Size, PBUF_REF);

if (p != NULL) {
p->payload = GetPayloadBuffer(Msg);
p->len = Msg->Size;
if (Interface->input(p, Interface) != ERR_OK ) {
pbuf_free(p); // On fail, we have to free the buffer
} else {
CallbackResult = 1;
// buffer is already freed internally
}
}

My Msg was actually a stack variable, I was assuming that Interface->input() was blocking and that after that call the content of that stack variable gets cloned. However this isn't the case especially with PBUF_REF set.

Benjamin K <bkausbk>
Thu 24 Sep 2020 06:49:31 AM UTC, comment #12: 

Maybe the time is better invested debugging the error you have instead of going to git head.

- What is the value of p or p->payload when it crashes?
- Are you sure threading requirements are met in your setup?
- Are you sure the STM MAC driver is not buggy?

I'm not aware of any problems here. Also, extended fuzzing tests did not show such bugs (and they showed quite a lot, lately).

Simon Goldschmidt <goldsimon>
Group administrator
Thu 24 Sep 2020 06:26:42 AM UTC, comment #11: 

I compared ip4_frag.c version 2.1.2 with git head and there were no relevant changes made. I ommited to further test that version since I already burned a full week in realizing that the bug was not in our product. I will put that on my todo list and if I get some time I will test it again.

Just want to inform the community that there seems to be a kind of problem.

Benjamin K <bkausbk>
Wed 23 Sep 2020 08:27:55 AM UTC, comment #10: 

Re comment #9:
You're using 2.1.2 but this report is set to "git head". Can you check if the issue is still present in git head? Thanks.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 23 Sep 2020 08:25:09 AM UTC, comment #9: 

I can confirm that bug. I'm using STM32F767II MCU with FreeRtos and LwIP 2.1.2. I'm not working with real hardware network interface but I've implemented a virtual network interface without any DMA access etc.

In ip_reass_free_complete_datagram:
[...]
  /* First, free all received pbufs.  The individual pbufs need to be released
     separately as they have not yet been chained */
  p = ipr->p;
  while (p != NULL) {
    struct pbuf *pcur;
->    iprh = (struct ip_reass_helper *)p->payload;

It "crashed" in the second iteration of that while-loop.

Setting IP_REASSEMBLY to 0 helps to prevent that fault, however that is not a solution. Are there any news about how to fix that

Benjamin K <bkausbk>
Sun 16 Feb 2020 07:44:35 PM UTC, comment #8: 

Changed the summary of task #14807 to include IPv4 reassemly, so this can be closed.

Also, I've added code to the windows port to test readonly RX.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 13 Feb 2020 07:55:24 PM UTC, comment #7: 

Yes, that's task #14807 (not a bug).

I'll still leave this open as it might hit others and I think IP reassembly and TCP might be the only parts that write to rx pbufs.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 13 Feb 2020 07:48:17 PM UTC, comment #6: 


> As to the read/write of memory: lwIP does write into the payload of received packets. They are not readonly. TCP does that (there's a bug that this might need to be changed) and IPv4 reassembly does that, too: it stores reassembly information in the pbuf where the IP header would be (after copying the header once).
>

This is good to know. I assumed otherwise, so this is something I need to get back to. I won't be able to check it right away so feel free to close this for now.

Can you point me to that tcp bug so I can follow up any discussion there?

Thanks

Jarno Malmari <jmalmari>
Wed 12 Feb 2020 09:25:36 AM UTC, comment #5: 


> Yes. My app is single threaded. The public example happened to be multi.


Your app is not the only thing that can cause problems. It's mainly the port (are interrupts used, how are timers executed, do interrupts call into lwIP code, etc) that causes problems.

As to the read/write of memory: lwIP does write into the payload of received packets. They are not readonly. TCP does that (there's a bug that this might need to be changed) and IPv4 reassembly does that, too: it stores reassembly information in the pbuf where the IP header would be (after copying the header once).

So this might be the fault you're seeing.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 12 Feb 2020 08:55:34 AM UTC, comment #4: 


> Are you at least sure it's not a threading issue?

Yes. My app is single threaded. The public example happened to be multi.

>
> If you could send a simple udp API level application to test against, I could try to test that on another target...
>

No udp api code is even needed. Simple loop running just the lwip stack is enough.

> > Is it ok for LWIP_RAM_HEAP_POINTER to be only for writing and RX only for reading?
>
> I'm afraid I don't understand that.

Sorry, I wasn't clear. I mean the lwip is given a pool that may be write only (that eth dma will read). For lwip input, a custom pbuf is allocated from external pool that may be read only (that eth dma wrote). I just wonder whether it's safe to assume lwip doesn't use pbuf payloads in a conflicting way under the hood.

Jarno Malmari <jmalmari>
Tue 11 Feb 2020 07:40:59 PM UTC, comment #3: 

Are you at least sure it's not a threading issue?

If you could send a simple udp API level application to test against, I could try to test that on another target...

> Is it ok for LWIP_RAM_HEAP_POINTER to be only for writing and RX only for reading?


I'm afraid I don't understand that.

But I would have thought the memory setup of the H7 itself should not be a problem, unless configured wrong.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 11 Feb 2020 07:22:46 PM UTC, comment #2: 

comment #1:

> You won't get me to reproduce this with an H7 board, sorry.


Understandable.

>
> The drivers ST provides are known to be buggy. Why are you sure this is a bug in lwIP, not in your port?


I am not. During my short exposure with ST drivers, I agree, they seem not well tested. However, this consistently fails on me while no other stress test does. On the other hand, I do have ST's F4 with LwIP that doesn't fail. Biggest difference between the two is the memory setup and layout.

Any restrictions on what type of memory access should be present for LwIP's rx/tx to work? Is it ok for LWIP_RAM_HEAP_POINTER to be only for writing and RX only for reading?

Jarno Malmari <jmalmari>
Tue 11 Feb 2020 06:28:41 PM UTC, comment #1: 

You won't get me to reproduce this with an H7 board, sorry.

The drivers ST provides are known to be buggy. Why are you sure this is a bug in lwIP, not in your port?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 11 Feb 2020 04:05:23 PM UTC, original submission:  

Test setup


Device under test

Ran into crash with custom ST H7 board. Reproducible with NUCLEO-H743ZI2.

Software


Reproducable with ST's project example:
STM32Cube_FW_H7_V1.6.0/Projects/NUCLEO-H743ZI/Applications/LwIP/LwIP_HTTP_Server_Netconn_RTOS

(only changed IP and disabled dhcp)

Available at
https://www.st.com/en/embedded-software/stm32cubeh7.html

LwIP


lwipopts.h attached

Versions tested: 2.0.3, 2.2.0

Not sure if relevant, but peculiarities of H7 include its multiple SRAMs. In the example, program data is in AXI SRAM to which ethernet DMA has no access. Therefore, the LWIP_RAM_HEAP_POINTER is relocated to SRAM3. Ethernet RX buffers are also in SRAM3. CPU cache for these memory regions is configured (disabled) by Memory Protection Unit (MPU_Config()).



Test run


From Linux shell:

bs=6000
ip=<device ip>
dd if=/dev/urandom bs=$bs count=5 |socat -b $bs -u stdin UDP4-DATAGRAM:$ip:12345


Expect hardfault after a few seconds.

Debug

Source of hardfault is function ip_reass_free_complete_datagram, dereferencing invalid pointer p (p->payload).

Issue seems to be combination of fragmented packets and receive buffer filling up. Therefore, depending on memory settings the amount of UDP bombardment may need adjusting.

Typical output with IP_REASS_DEBUG=LWIP_DBG_ON:

ip_reass_pbufcount: 1 out
ip4_reass: matching previous fragment ID=8778
ip4_reass: last fragment seen, total len 1508
ip_reass_pbufcount: 2 out
ip_reass_tmr: timer dec 14
ip_reass_pbufcount: 3 out
ip4_reass: matching previous fragment ID=8779
ip4_reass: last fragment seen, total len 1508
ip_reass_pbufcount: 4 out
ip_reass_pbufcount: 5 out
ip4_reass: matching previous fragment ID=877a
ip4_reass: last fragment seen, total len 1508
ip_reass_pbufcount: 6 out
ip4_reass: last fragment seen, total len 1508
ip_reass_pbufcount: 7 out
ip4_reass: last fragment seen, total len 1508
ip_reass_pbufcount: 8 out
ip_reass_tmr: timer dec 14
ip_reass_tmr: timer dec 14
ip_reass_tmr: timer dec 14
ip_reass_tmr: timer dec 14
ip_reass_tmr: timer dec 13
ip_reass_tmr: timer dec 13
ip_reass_tmr: timer dec 13
ip_reass_tmr: timer dec 13
<...counting down from 13 to 1...>
ip_reass_tmr: timer dec 1
ip_reass_tmr: timer dec 1
ip_reass_tmr: timer dec 0
ip_reass_tmr: timer dec 0
ip_reass_tmr: timer dec 0
ip_reass_tmr: timer dec 0
ip_reass_tmr: timer dec 0
ip_reass_tmr: timer timed out
<hardfault>


Workaround


#define IP_REASSEMBLY 0


Jarno Malmari <jmalmari>

 

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

Attached Files
file #48393:  lwipopts.h added by jmalmari (9KiB - text/x-chdr)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bkausbk (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jmalmari (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
    2020-02-16 goldsimon StatusNone Invalid
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2020-02-11 jmalmari Attached File- Added lwipopts.h, #48393

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code