From 737ed228c96feb6a0caa3255570252f14f38f547 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Fri, 28 Jul 2017 09:41:00 +0200 Subject: [PATCH] ip4_reass: fixed bug #51595 (ip_reass_pbufcount may be updated incorectly) (cherry picked from commit f1072fee8ab171b4b878b3718bf1c7a4cf340e1d) # Conflicts: # src/core/ipv4/ip4_frag.c --- src/core/ipv4/ip4_frag.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/core/ipv4/ip4_frag.c b/src/core/ipv4/ip4_frag.c index 57fb44cb..787e384d 100644 --- a/src/core/ipv4/ip4_frag.c +++ b/src/core/ipv4/ip4_frag.c @@ -79,6 +79,10 @@ #define IP_REASS_FLAG_LASTFRAG 0x01 +#define IP_REASS_VALIDATE_TELEGRAM_FINISHED 1 +#define IP_REASS_VALIDATE_PBUF_QUEUED 0 +#define IP_REASS_VALIDATE_PBUF_DROPPED -1 + /** This is a helper struct which holds the starting * offset and the ending offset of this fragment to * easily chain the fragments. @@ -333,7 +337,7 @@ ip_reass_dequeue_datagram(struct ip_reassdata *ipr, struct ip_reassdata *prev) * fragment was received at least once). * @param ipr points to the reassembly state * @param new_p points to the pbuf for the current fragment - * @return 0 if invalid, >0 otherwise + * @return see IP_REASS_VALIDATE_* defines */ static int ip_reass_chain_frag_into_datagram_and_validate(struct ip_reassdata *ipr, struct pbuf *new_p) @@ -462,15 +466,15 @@ ip_reass_chain_frag_into_datagram_and_validate(struct ip_reassdata *ipr, struct /* If valid is 0 here, there are some fragments missing in the middle * (since MF == 0 has already arrived). Such datagrams simply time out if * no more fragments are received... */ - return valid; + return valid ? IP_REASS_VALIDATE_TELEGRAM_FINISHED : IP_REASS_VALIDATE_PBUF_QUEUED; } /* If we come here, not all fragments were received, yet! */ - return 0; /* not yet valid! */ + return IP_REASS_VALIDATE_PBUF_QUEUED; /* not yet valid! */ #if IP_REASS_CHECK_OVERLAP freepbuf: ip_reass_pbufcount -= pbuf_clen(new_p); pbuf_free(new_p); - return 0; + return IP_REASS_VALIDATE_PBUF_DROPPED; #endif /* IP_REASS_CHECK_OVERLAP */ } @@ -488,6 +492,7 @@ ip4_reass(struct pbuf *p) struct ip_reassdata *ipr; struct ip_reass_helper *iprh; u16_t offset, len, clen; + int valid; IPFRAG_STATS_INC(ip_frag.recv); MIB2_STATS_INC(mib2.ipreasmreqds); @@ -552,9 +557,6 @@ ip4_reass(struct pbuf *p) SMEMCPY(&ipr->iphdr, fraghdr, IP_HLEN); } } - /* Track the current number of pbufs current 'in-flight', in order to limit - the number of fragments that may be enqueued at any one time */ - ip_reass_pbufcount += clen; /* At this point, we have either created a new entry or pointing * to an existing one */ @@ -569,7 +571,18 @@ ip4_reass(struct pbuf *p) } /* find the right place to insert this pbuf */ /* @todo: trim pbufs if fragments are overlapping */ - if (ip_reass_chain_frag_into_datagram_and_validate(ipr, p)) { + valid = ip_reass_chain_frag_into_datagram_and_validate(ipr, p); + if (valid == IP_REASS_VALIDATE_PBUF_DROPPED) { + goto nullreturn; + } + /* if we come here, the pbuf has been enqueued */ + + /* Track the current number of pbufs current 'in-flight', in order to limit + the number of fragments that may be enqueued at any one time + (overflow checked by testing against IP_REASS_MAX_PBUFS) */ + ip_reass_pbufcount = (u16_t)(ip_reass_pbufcount + clen); + + if (valid == IP_REASS_VALIDATE_TELEGRAM_FINISHED) { struct ip_reassdata *ipr_prev; /* the totally last fragment (flag more fragments = 0) was received at least * once AND all fragments are received */