QUERY · ISSUE
[ESP32 port] network_ppp does not manage pcb properly; can cause LWIP assertion or unmanaged pcb
bug
In handling of active(false), network_ppp.c ignores the return value of pppapi_free() and always sets self->pcb to NULL. However, pppapi_free() can fail if the PPP link is not currently dead, in which case the PCB remains allocated.
The implication of this is:
- The PCB is now freed and yet network_ppp.c no longer has a reference to it. The PCB is essentially 'leaked' at this point.
- Subsequent attempts to call active(true) will cause an LWIP assertion (which leads to a panic) due to the fact that the network interface we are trying to add already exists (due to the previously-unfreed pcb).
- Also note that since the pcb didn't get freed, it's possible that the ppp_status_cb() function will be called by LWIP. If it does, the fact that self->pcb is set to NULL can cause an Access Violation (or Load Prohibited) since the ppp_status_cb() function assumes self->pcb is non-NULL.
The scenario above is exacerbated by the fact that:
- network_ppp.c has a 4000 ms timeout for the pppapi_close() call. However, if pppapi_close() was called while the LCP finite state machine had not yet set the phase of the PPP to DEAD, then LCP layer will try to send 2 TERM_REQs to its peer.
- If the peer is not answering, both of these TERM_REQs will timeout. In the version of LWIP, the timeout is configured for 6 seconds per attempt, for a total of 12 seconds. Therefore the LCP layer will require more than the 4 (or 4000ms) given by network_ppp.c.
- As a result, active(false) will timeout on pppapi_close(). Furthermore its call to pppapi_free() will fail and this failure will trigger the initial problem described above where network_ppp.c has NULLed its reference to the PCB eventhough the PCB hasn't been freed successfully in LWIP.
CANDIDATE · PULL REQUEST
esp32/network_ppp: added ppp_set_usepeerdns(self->pcb, 1);
port-esp32
Without this, you often don't get any DNS server from your network provider. Additionally,
setting your own DNS does not work without this option set; this is almost certainly
a bug in the PPP stack.