mirror/lwip
1
0
Fork 0
mirror of https://github.com/lwip-tcpip/lwip.git synced 2024-10-05 22:29:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

PPP, PPPoS, improve thread safety of pppos_input()

Browse Source 
Don't process input data if PPPoS is closed, it helps using
pppos_input() from a different context to prevent pppos_input() to
modify PPPoS RX machine state on a closed PPPoS session. It also
prevents allocating pbuf (which are going to be tossed out by PPP core)
and parsing serial input on a closed session.

It only mitigates the fact that this function is actually NOT thread
safe in absolutely all cases, it does not fix it but it helps for a low
cost.

For example user application should never call pppos_input() while
pppos_connect() or pppos_listen() is currently running because both of
them are freeing any input pbuf left over from the last session before
resetting the PPPoS state, they really have to to prevent pbuf leaks.

We cannot fix that easily because we don't have spinlock with an
irqsave/irqrestore helper for IRQ contexts. Mutex cannot be used in
interrupt contexts (or again, with an IRQ mutex helper).

We are going to improve the documentation on this point.
This commit is contained in:
Sylvain Rochet 2015-03-10 22:52:35 +01:00
parent d5cbacba50
commit d518f5f307
2 changed files with 19 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/include/netif/ppp/pppos.h
View File

@ -76,6 +76,10 @@ struct pppos_pcb_s {
*/
ext_accm out_accm; /* Async-Ctl-Char-Map for output. */
/* flags */
unsigned int open :1; /* Set if PPPoS is open */
unsigned int :7; /* 7 bits of padding to round out to 8 bits */
/* PPPoS rx */
ext_accm in_accm; /* Async-Ctl-Char-Map for input. */
struct pbuf *in_head, *in_tail; /* The input packet. */

16
src/netif/ppp/pppos.c
View File

@ -409,6 +409,7 @@ pppos_connect(ppp_pcb *ppp, void *ctx)
*/
pppos->in_accm[15] = 0x60; /* no need to protect since RX is not running */
pppos->out_accm[15] = 0x60;
pppos->open = 1;
/*
* Start the connection and handle incoming events (packet or timeout).
@ -468,6 +469,7 @@ pppos_listen(ppp_pcb *ppp, void *ctx, struct ppp_addrs *addrs)
*/
pppos->in_accm[15] = 0x60; /* no need to protect since RX is not running */
pppos->out_accm[15] = 0x60;
pppos->open = 1;
/*
* Wait for something to happen.
@ -481,7 +483,12 @@ pppos_listen(ppp_pcb *ppp, void *ctx, struct ppp_addrs *addrs)
static void
pppos_disconnect(ppp_pcb *ppp, void *ctx)
{
LWIP_UNUSED_ARG(ctx);
pppos_pcb *pppos = (pppos_pcb *)ctx;
PPPOS_DECL_PROTECT(lev);
PPPOS_PROTECT(lev);
pppos->open = 0;
PPPOS_UNPROTECT(lev);
/* We cannot call pppos_free_current_input_packet() here because
* rx thread might still call pppos_input()
@ -534,6 +541,13 @@ pppos_input(ppp_pcb *ppp, u_char *s, int l)
u_char escaped;
PPPOS_DECL_PROTECT(lev);
PPPOS_PROTECT(lev);
if (!pppos->open) {
PPPOS_UNPROTECT(lev);
return;
}
PPPOS_UNPROTECT(lev);
PPPDEBUG(LOG_DEBUG, ("pppos_input[%d]: got %d bytes\n", ppp->netif->num, l));
while (l-- > 0) {
cur_char = *s++;