Menu

#1009 Incorrect pointer type in lwipthread.c low_level_input causes panic on packet reception

trunk
closed
None
lwIP
Medium
Trunk
True
2020-03-19
2019-02-04
Adam
No

Since 10790 (integrating lwip 2.0.3), lwipthread.c L196 low_level_input() has a struct pbuf **pbuf instead of a struct pbuf *p but it continues to call pbuf_header(pbuf, ETH_PAD_SIZE) where it used to call pbuf_header(p, ETH_PAD_SIZE). Since the types are wrong this is invalid.

This generates a compiler warning about mismatched pointer types:

../ChibiOS/os/various/lwip_bindings/lwipthread.c: In function 'low_level_input':
../ChibiOS/os/various/lwip_bindings/lwipthread.c:196:17: warning: passing argument 1 of 'pbuf_header' from incompatible pointer type [-Wincompatible-pointer-types]
     pbuf_header(pbuf, -ETH_PAD_SIZE); /* drop the padding word */
                 ^~~~
In file included from ../ChibiOS/os/various/lwip_bindings/lwipthread.c:66:0:
../ChibiOS/ext/lwip/src/include/lwip/pbuf.h:230:6: note: expected 'struct pbuf *' but argument is of type 'struct pbuf **'
 u8_t pbuf_header(struct pbuf *p, s16_t header_size);
      ^~~~~~~~~~~
      ```

      but more importantly this also causes a panic as soon as a packet is received:

      ```
      0x08011386 in chSysHalt (reason=reason@entry=0x802022c "increment_magnitude <= p->len")
    at ../ChibiOS/os/rt/src/chsys.c:206
206   ch.dbg.panic_msg = reason;
(gdb) p reason
$1 = 0x802022c "increment_magnitude <= p->len"
(gdb) bt
#0  0x08011386 in chSysHalt (reason=reason@entry=0x802022c "increment_magnitude <= p->len")
    at ../ChibiOS/os/rt/src/chsys.c:206
#1  0x080165cc in osalSysHalt (reason=0x802022c "increment_magnitude <= p->len")
    at ../ChibiOS/os/hal/osal/rt/osal.h:480
#2  pbuf_header_impl (p=<optimised out>, header_size_increment=<optimised out>, force=force@entry=0 '\000')
    at ../ChibiOS/ext/lwip/src/core/pbuf.c:583
#3  0x080166a4 in pbuf_header_impl (force=0 '\000', header_size_increment=header_size_increment@entry=-2,
    p=p@entry=0x2000980c <wa_lwip_thread+676>) at ../ChibiOS/ext/lwip/src/core/pbuf.c:668
#4  pbuf_header (p=p@entry=0x2000989c <wa_lwip_thread+820>, header_size_increment=header_size_increment@entry=-2)
    at ../ChibiOS/ext/lwip/src/core/pbuf.c:667
#5  0x08019570 in low_level_input (netif=0x20009534 <thisif>, pbuf=0x2000989c <wa_lwip_thread+820>)
    at ../ChibiOS/os/various/lwip_bindings/lwipthread.c:196
#6  lwip_thread (p=<optimised out>) at ../ChibiOS/os/various/lwip_bindings/lwipthread.c:387
#7  0x0801032e in _port_thread_start () at ../ChibiOS/os/common/ports/ARMCMx/compilers/GCC/chcoreasm_v7m.S:119
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

The fix is to dereference pbuf, i.e., call pbuf_header(*pbuf, ETH_PAD_SIZE) both on L196 and again on L216 (the two calls to pbuf_header() in low_level_input()).

This seems to have been the case since Oct 2017 so I'm not really sure how it's gone unnoticed unless it only happens with specific configuration options?

Discussion

  • Adam

    Adam - 2019-02-04

    Apologies for slightly butchered formatting in the middle there. Should have been two separate code blocks.

     
  • Adam

    Adam - 2020-01-08

    Is there any change of getting this fixed soon? I think it's a pretty straightforward one line fix, but it's still causing runtime panics for me whenever I enable ETH_PAD_SIZE to get good IP address alignment.

     
  • Giovanni Di Sirio

    Hi, I am meging improvements in lwIP, that bug is fixed as well.

     
  • Giovanni Di Sirio

    • Fixed in Repository: False --> True
     
  • Giovanni Di Sirio

    Fixed in all branches, there were several other errors, not just that one.

     
  • Giovanni Di Sirio

    • status: open --> closed
    • assigned_to: Giovanni Di Sirio
     
  • Giovanni Di Sirio

    Fixed in all branches, there were several other errors, not just that one.

     

Log in to post a comment.