From a39ce0f53b2a57c4e7fbb252efb579a629201868 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Mon, 19 Oct 2020 20:22:05 +0200 Subject: [PATCH] PPP, PPPoS: drop input packets much bigger than our MRU Our current HDLC decoder does not protect against starving the Rx PBUF POOL for one packet, most likely due to received garbage on the serial port. Prevent starving the Rx pool by checking incoming packets length against PPP_MRU with a 10% margin because we only want to avoid filling all PBUFs with garbage, we don't have to be pedantic. Fixes bug #58441: Invalid PPP data accumulates forever. --- src/netif/ppp/pppos.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/netif/ppp/pppos.c b/src/netif/ppp/pppos.c index 88a4503a..9d3fc2a4 100644 --- a/src/netif/ppp/pppos.c +++ b/src/netif/ppp/pppos.c @@ -631,12 +631,37 @@ pppos_input(ppp_pcb *ppp, u8_t *s, int l) if (pppos->in_tail == NULL || pppos->in_tail->len == PBUF_POOL_BUFSIZE) { u16_t pbuf_alloc_len; if (pppos->in_tail != NULL) { + u16_t mru; pppos->in_tail->tot_len = pppos->in_tail->len; if (pppos->in_tail != pppos->in_head) { pbuf_cat(pppos->in_head, pppos->in_tail); /* give up the in_tail reference now */ pppos->in_tail = NULL; } + /* Compute MRU including headers length. If smaller packets are + * requested, we must still be able to receive packets of the + * default MRU for control packets. */ + mru = LWIP_MAX(PPP_MRU, PPP_DEFMRU) + /* Add 10% more. We only want to avoid filling all PBUFs with garbage, + * we don't have to be pedantic. */ + + LWIP_MAX(PPP_MRU, PPP_DEFMRU)/10 +#if IP_FORWARD || LWIP_IPV6_FORWARD + + PBUF_LINK_ENCAPSULATION_HLEN + PBUF_LINK_HLEN +#endif /* IP_FORWARD || LWIP_IPV6_FORWARD */ +#if PPP_INPROC_IRQ_SAFE + + sizeof(struct pppos_input_header) +#endif /* PPP_INPROC_IRQ_SAFE */ + + sizeof(pppos->in_protocol); + if (pppos->in_head->tot_len > mru) { + /* Packet too big. Drop the input packet and let the + * higher layers deal with it. Continue processing + * received characters in case a new packet starts. */ + PPPDEBUG(LOG_ERR, ("pppos_input[%d]: packet too big, max_len=%d, dropping packet\n", ppp->netif->num, mru)); + LINK_STATS_INC(link.lenerr); + pppos_input_drop(pppos); + pppos->in_state = PDIDLE; /* Wait for flag character. */ + break; + } } /* If we haven't started a packet, we need a packet header. */ pbuf_alloc_len = 0; @@ -653,7 +678,7 @@ pppos_input(ppp_pcb *ppp, u8_t *s, int l) if (next_pbuf == NULL) { /* No free buffers. Drop the input packet and let the * higher layers deal with it. Continue processing - * the received pbuf chain in case a new packet starts. */ + * received characters in case a new packet starts. */ PPPDEBUG(LOG_ERR, ("pppos_input[%d]: NO FREE PBUFS!\n", ppp->netif->num)); LINK_STATS_INC(link.memerr); pppos_input_drop(pppos);