PPP, PPPoS, fixed thread safety of pppos_input()

PPPoS was actually not thread safe, pppos_input() can be called from
lwIP user port at any time, whatever the PPP state is. It might even be
called during pppos_connect() and pppos_listen(), this is quite unlikely
the port do that but nothing prevent the user to since we document
pppos_input() as being thread safe.

Added a mutex if PPP_INPROC_MULTITHREADED is set and ensure pppos_input()
is safe in regard to other pppos_* functions.
This commit is contained in:
Sylvain Rochet 2015-03-08 02:48:52 +01:00
parent 7857c22277
commit ea58a8103c
2 changed files with 63 additions and 19 deletions

View File

@ -67,6 +67,9 @@ typedef struct pppos_pcb_s pppos_pcb;
struct pppos_pcb_s {
/* -- below are data that will NOT be cleared between two sessions */
ppp_pcb *ppp; /* PPP PCB */
#if PPP_INPROC_MULTITHREADED
sys_mutex_t mutex; /* Mutex for pppos_input() */
#endif /* PPP_INPROC_MULTITHREADED */
sio_fd_t fd; /* File device ID of port. */
/* -- below are data that will be cleared between two sessions

View File

@ -180,19 +180,33 @@ ppp_pcb *pppos_create(struct netif *pppif, sio_fd_t fd,
ppp = ppp_new(pppif, link_status_cb, ctx_cb);
if (ppp == NULL) {
return NULL;
goto ppp_new_fail;
}
pppos = (pppos_pcb *)memp_malloc(MEMP_PPPOS_PCB);
if (pppos == NULL) {
ppp_free(ppp);
return NULL;
goto pppos_malloc_fail;
}
#if PPP_INPROC_MULTITHREADED
if (sys_mutex_new(&pppos->mutex) != ERR_OK) {
goto mutex_new_fail;
}
#endif /* PPP_INPROC_MULTITHREADED */
pppos->ppp = ppp;
pppos->fd = fd;
ppp_link_set_callbacks(ppp, &pppos_callbacks, pppos);
return ppp;
#if PPP_INPROC_MULTITHREADED
mutex_new_fail:
memp_free(MEMP_PPPOS_PCB, pppos);
#endif /* PPP_INPROC_MULTITHREADED */
pppos_malloc_fail:
ppp_free(ppp);
ppp_new_fail:
return NULL;
}
/* Called by PPP core */
@ -387,12 +401,18 @@ pppos_connect(ppp_pcb *ppp, void *ctx)
ipcp_options *ipcp_ao;
#endif /* !VJ_SUPPORT && PPP_IPV4_SUPPORT */
/* input pbuf left over from last session? */
#if PPP_INPROC_MULTITHREADED
sys_mutex_lock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
/* input pbuf left ? */
pppos_free_current_input_packet(pppos);
ppp_clear(ppp);
/* reset PPPoS control block to its initial state */
memset(&pppos->out_accm, 0, sizeof(pppos_pcb) - ( (char*)&((pppos_pcb*)0)->out_accm - (char*)0 ) );
#if PPP_INPROC_MULTITHREADED
sys_mutex_unlock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
ppp_clear(ppp);
#if PPP_IPV4_SUPPORT
#if VJ_SUPPORT
@ -437,12 +457,18 @@ pppos_listen(ppp_pcb *ppp, void *ctx, struct ppp_addrs *addrs)
#endif /* PPP_IPV4_SUPPORT */
lcp_options *lcp_wo;
/* input pbuf left over from last session? */
#if PPP_INPROC_MULTITHREADED
sys_mutex_lock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
/* input pbuf left ? */
pppos_free_current_input_packet(pppos);
ppp_clear(ppp);
/* reset PPPoS control block to its initial state */
memset(&pppos->out_accm, 0, sizeof(pppos_pcb) - ( (char*)&((pppos_pcb*)0)->out_accm - (char*)0 ) );
#if PPP_INPROC_MULTITHREADED
sys_mutex_unlock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
ppp_clear(ppp);
/* Wait passively */
lcp_wo = &ppp->lcp_wantoptions;
@ -499,11 +525,17 @@ 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;
/* input pbuf left ? */
#if PPP_INPROC_MULTITHREADED
sys_mutex_lock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
pppos_free_current_input_packet(pppos);
#if PPP_INPROC_MULTITHREADED
sys_mutex_unlock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
/* We cannot call pppos_free_current_input_packet() here because
* rx thread might still call pppos_input()
*/
ppp_link_end(ppp); /* notify upper layers */
}
@ -516,6 +548,9 @@ pppos_destroy(ppp_pcb *ppp, void *ctx)
/* input pbuf left ? */
pppos_free_current_input_packet(pppos);
#if PPP_INPROC_MULTITHREADED
sys_mutex_free(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
memp_free(MEMP_PPPOS_PCB, pppos);
return ERR_OK;
}
@ -550,15 +585,15 @@ pppos_input(ppp_pcb *ppp, u_char *s, int l)
struct pbuf *next_pbuf;
u_char cur_char;
u_char escaped;
SYS_ARCH_DECL_PROTECT(lev);
PPPDEBUG(LOG_DEBUG, ("pppos_input[%d]: got %d bytes\n", ppp->netif->num, l));
#if PPP_INPROC_MULTITHREADED
sys_mutex_lock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
while (l-- > 0) {
cur_char = *s++;
SYS_ARCH_PROTECT(lev);
escaped = ESCAPE_P(pppos->in_accm, cur_char);
SYS_ARCH_UNPROTECT(lev);
/* Handle special characters. */
if (escaped) {
/* Check for escape sequences. */
@ -762,6 +797,9 @@ pppos_input(ppp_pcb *ppp, u_char *s, int l)
pppos->in_fcs = PPP_FCS(pppos->in_fcs, cur_char);
}
} /* while (l-- > 0), all bytes processed */
#if PPP_INPROC_MULTITHREADED
sys_mutex_unlock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
magic_randomize();
}
@ -812,15 +850,18 @@ pppos_recv_config(ppp_pcb *ppp, void *ctx, u32_t accm)
{
int i;
pppos_pcb *pppos = (pppos_pcb *)ctx;
SYS_ARCH_DECL_PROTECT(lev);
LWIP_UNUSED_ARG(ppp);
/* Load the ACCM bits for the 32 control codes. */
SYS_ARCH_PROTECT(lev);
#if PPP_INPROC_MULTITHREADED
sys_mutex_lock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
for (i = 0; i < 32 / 8; i++) {
pppos->in_accm[i] = (u_char)(accm >> (i * 8));
}
SYS_ARCH_UNPROTECT(lev);
#if PPP_INPROC_MULTITHREADED
sys_mutex_unlock(&pppos->mutex);
#endif /* PPP_INPROC_MULTITHREADED */
PPPDEBUG(LOG_INFO, ("pppos_recv_config[%d]: in_accm=%X %X %X %X\n",
pppos->ppp->netif->num,