diff --git a/CHANGELOG b/CHANGELOG index 5adb50e0..6597dee0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -241,6 +241,12 @@ HISTORY ++ Bug fixes: + 2007-07-03 Simon Goldschmidt + * nearly-all-files: Added assertions where PBUF_RAM pbufs are used and an + assumption is made that this pbuf is in one piece (i.e. not chained). These + assumptions clash with the possibility of converting to fully pool-based + pbuf implementations, where PBUF_RAM pbufs might be chained. + 2007-07-03 Simon Goldschmidt * api.h, api_lib.c, api_msg.c: Final fix for bug #20021 and some other problems when closing tcp netconns: removed conn->sem, less context switches when diff --git a/src/api/api_lib.c b/src/api/api_lib.c index d696ff28..557aa4c0 100644 --- a/src/api/api_lib.c +++ b/src/api/api_lib.c @@ -108,6 +108,8 @@ netbuf_alloc(struct netbuf *buf, u16_t size) if (buf->p == NULL) { return NULL; } + LWIP_ASSERT("check that first pbuf can hold size", + (buf->p->len >= size)); buf->ptr = buf->p; return buf->p->payload; } diff --git a/src/core/dhcp.c b/src/core/dhcp.c index eaa3a12a..94e3fe30 100644 --- a/src/core/dhcp.c +++ b/src/core/dhcp.c @@ -1312,6 +1312,9 @@ static err_t dhcp_create_request(struct netif *netif) LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("dhcp_create_request(): could not allocate pbuf\n")); return ERR_MEM; } + LWIP_ASSERT("check that first pbuf can hold struct dhcp_msg", + (dhcp->p_out->len >= sizeof(struct dhcp_msg))); + /* give unique transaction identifier to this request */ dhcp->xid = xid++; LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("transaction id xid++(%"X32_F") dhcp->xid(%"U32_F")\n",xid,dhcp->xid)); diff --git a/src/core/ipv4/icmp.c b/src/core/ipv4/icmp.c index 8fb07715..3082fb00 100644 --- a/src/core/ipv4/icmp.c +++ b/src/core/ipv4/icmp.c @@ -112,6 +112,8 @@ icmp_input(struct pbuf *p, struct netif *inp) LWIP_DEBUGF(ICMP_DEBUG, ("icmp_input: allocating new pbuf failed\n")); goto memerr; } + LWIP_ASSERT("check that first pbuf can hold struct the ICMP header", + (r->len >= hlen + sizeof(struct icmp_echo_hdr))); /* copy the whole packet including ip header */ if (pbuf_copy(r, p) != ERR_OK) { LWIP_ASSERT("icmp_input: copying to new pbuf failed\n", 0); @@ -201,8 +203,16 @@ icmp_dest_unreach(struct pbuf *p, enum icmp_dur_type t) struct ip_hdr *iphdr; struct icmp_dur_hdr *idur; + /* @todo: can this be PBUF_LINK instead of PBUF_IP? */ q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM); /* ICMP header + IP header + 8 bytes of data */ + if (q == NULL) { + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n")); + pbuf_free(p); + return; + } + LWIP_ASSERT("check that first pbuf can hold icmp message", + (q->len >= (8 + IP_HLEN + 8))); iphdr = p->payload; @@ -240,7 +250,16 @@ icmp_time_exceeded(struct pbuf *p, enum icmp_te_type t) struct ip_hdr *iphdr; struct icmp_te_hdr *tehdr; + /* @todo: can this be PBUF_LINK instead of PBUF_IP? */ q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM); + /* ICMP header + IP header + 8 bytes of data */ + if (q == NULL) { + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded: failed to allocate pbuf for ICMP packet.\n")); + pbuf_free(p); + return; + } + LWIP_ASSERT("check that first pbuf can hold icmp message", + (q->len >= (8 + IP_HLEN + 8))); iphdr = p->payload; LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded from ")); diff --git a/src/core/ipv4/igmp.c b/src/core/ipv4/igmp.c index 204fdc54..bc05f454 100644 --- a/src/core/ipv4/igmp.c +++ b/src/core/ipv4/igmp.c @@ -562,6 +562,8 @@ igmp_send(struct igmp_group *group, u8_t type) if (p) { igmp = p->payload; + LWIP_ASSERT("check that first pbuf can hold struct igmpmsg", + (p->len >= sizeof(struct igmpmsg))); ip_addr_set(&src, &((group->interface)->ip_addr)); if (IGMP_V2_MEMB_REPORT == type) { diff --git a/src/core/ipv4/ip.c b/src/core/ipv4/ip.c index 3fc59a34..43263953 100644 --- a/src/core/ipv4/ip.c +++ b/src/core/ipv4/ip.c @@ -460,6 +460,8 @@ ip_output_if(struct pbuf *p, struct ip_addr *src, struct ip_addr *dest, } iphdr = p->payload; + LWIP_ASSERT("check that first pbuf can hold struct ip_hdr", + (p->len >= sizeof(struct ip_hdr))); IPH_TTL_SET(iphdr, ttl); IPH_PROTO_SET(iphdr, proto); diff --git a/src/core/ipv4/ip_frag.c b/src/core/ipv4/ip_frag.c index 9a2345e4..e20f70bb 100644 --- a/src/core/ipv4/ip_frag.c +++ b/src/core/ipv4/ip_frag.c @@ -403,6 +403,8 @@ ip_frag(struct pbuf *p, struct netif *netif, struct ip_addr *dest) if (rambuf == NULL) { return ERR_MEM; } + LWIP_ASSERT("this needs a pbuf in one piece!", + (p->len >= (IP_HLEN))); SMEMCPY(rambuf->payload, original_iphdr, IP_HLEN); iphdr = rambuf->payload; diff --git a/src/core/ipv6/icmp6.c b/src/core/ipv6/icmp6.c index f80ca990..6e49295f 100644 --- a/src/core/ipv6/icmp6.c +++ b/src/core/ipv6/icmp6.c @@ -121,8 +121,16 @@ icmp_dest_unreach(struct pbuf *p, enum icmp_dur_type t) struct ip_hdr *iphdr; struct icmp_dur_hdr *idur; + /* @todo: can this be PBUF_LINK instead of PBUF_IP? */ q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM); /* ICMP header + IP header + 8 bytes of data */ + if (q == NULL) { + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n")); + pbuf_free(p); + return; + } + LWIP_ASSERT("check that first pbuf can hold icmp message", + (q->len >= (8 + IP_HLEN + 8))); iphdr = p->payload; @@ -153,7 +161,16 @@ icmp_time_exceeded(struct pbuf *p, enum icmp_te_type t) LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded\n")); + /* @todo: can this be PBUF_LINK instead of PBUF_IP? */ q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM); + /* ICMP header + IP header + 8 bytes of data */ + if (q == NULL) { + LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n")); + pbuf_free(p); + return; + } + LWIP_ASSERT("check that first pbuf can hold icmp message", + (q->len >= (8 + IP_HLEN + 8))); iphdr = p->payload; diff --git a/src/core/pbuf.c b/src/core/pbuf.c index ea79adc3..5dbdfb57 100644 --- a/src/core/pbuf.c +++ b/src/core/pbuf.c @@ -153,7 +153,7 @@ pbuf_alloc(pbuf_layer l, u16_t length, pbuf_flag flag) switch (flag) { case PBUF_POOL: /* allocate head of pbuf chain into p */ - p = memp_malloc(MEMP_PBUF_POOL); + p = memp_malloc(MEMP_PBUF_POOL); LWIP_DEBUGF(PBUF_DEBUG | LWIP_DBG_TRACE | 3, ("pbuf_alloc: allocated pbuf %p\n", (void *)p)); if (p == NULL) { return NULL; @@ -184,7 +184,7 @@ pbuf_alloc(pbuf_layer l, u16_t length, pbuf_flag flag) rem_len = length - p->len; /* any remaining pbufs to be allocated? */ while (rem_len > 0) { - q = memp_malloc(MEMP_PBUF_POOL); + q = memp_malloc(MEMP_PBUF_POOL); if (q == NULL) { /* free chain so far allocated */ pbuf_free(p); diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c index f0df636a..812bb80b 100644 --- a/src/core/tcp_out.c +++ b/src/core/tcp_out.c @@ -214,6 +214,8 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len, if ((seg->p = pbuf_alloc(PBUF_TRANSPORT, optlen, PBUF_RAM)) == NULL) { goto memerr; } + LWIP_ASSERT("check that first pbuf can hold optlen", + (seg->p->len >= optlen)); ++queuelen; seg->dataptr = seg->p->payload; } @@ -223,6 +225,8 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len, LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 2, ("tcp_enqueue : could not allocate memory for pbuf copy size %"U16_F"\n", seglen)); goto memerr; } + LWIP_ASSERT("check that first pbuf can hold the complete seglen", + (seg->p->len >= seglen)); ++queuelen; if (arg != NULL) { MEMCPY(seg->p->payload, ptr, seglen); @@ -664,6 +668,8 @@ tcp_rst(u32_t seqno, u32_t ackno, LWIP_DEBUGF(TCP_DEBUG, ("tcp_rst: could not allocate memory for pbuf\n")); return; } + LWIP_ASSERT("check that first pbuf can hold struct tcp_hdr", + (p->len >= sizeof(struct tcp_hdr))); tcphdr = p->payload; tcphdr->src = htons(local_port); @@ -784,6 +790,8 @@ tcp_keepalive(struct tcp_pcb *pcb) LWIP_DEBUGF(TCP_DEBUG, ("tcp_keepalive: could not allocate memory for pbuf\n")); return; } + LWIP_ASSERT("check that first pbuf can hold struct tcp_hdr", + (p->len >= sizeof(struct tcp_hdr))); tcphdr = p->payload; tcphdr->src = htons(pcb->local_port); diff --git a/src/core/udp.c b/src/core/udp.c index 391df770..e187a52a 100644 --- a/src/core/udp.c +++ b/src/core/udp.c @@ -379,6 +379,8 @@ udp_send(struct udp_pcb *pcb, struct pbuf *p) q = p; LWIP_DEBUGF(UDP_DEBUG, ("udp_send: added header in given pbuf %p\n", (void *)p)); } + LWIP_ASSERT("check that first pbuf can hold struct udp_hdr", + (q->len >= sizeof(struct udp_hdr))); /* q now represents the packet to be sent */ udphdr = q->payload; udphdr->src = htons(pcb->local_port); diff --git a/src/netif/etharp.c b/src/netif/etharp.c index fa379b6a..cc0ed30b 100644 --- a/src/netif/etharp.c +++ b/src/netif/etharp.c @@ -1028,6 +1028,8 @@ etharp_raw(struct netif *netif, const struct eth_addr *ethsrc_addr, LWIP_DEBUGF(ETHARP_DEBUG | LWIP_DBG_TRACE | 2, ("etharp_raw: could not allocate pbuf for ARP request.\n")); return ERR_MEM; } + LWIP_ASSERT("check that first pbuf can hold struct etharp_hdr", + (p->len >= sizeof(struct etharp_hdr))); hdr = p->payload; LWIP_DEBUGF(ETHARP_DEBUG | LWIP_DBG_TRACE, ("etharp_raw: sending raw ARP packet.\n")); diff --git a/src/netif/loopif.c b/src/netif/loopif.c index cec0845d..32037aac 100644 --- a/src/netif/loopif.c +++ b/src/netif/loopif.c @@ -61,7 +61,7 @@ void loopif_poll(struct netif *netif) { SYS_ARCH_DECL_PROTECT(lev); - struct pbuf *in = NULL; + struct pbuf *in, *in_end; struct loopif_private *priv = (struct loopif_private*)netif->state; LWIP_ERROR("priv != NULL", (priv != NULL), return;); @@ -70,30 +70,35 @@ loopif_poll(struct netif *netif) /* Get a packet from the list. With SYS_LIGHTWEIGHT_PROT=1, this is protected */ SYS_ARCH_PROTECT(lev); in = priv->first; - if(priv->first) { - if(priv->first == priv->last) { + if(in) { + in_end = in; + while(in_end->len != in_end->tot_len) { + LWIP_ASSERT("bogus pbuf: len != tot_len but next == NULL!", in_end->next != NULL); + in_end = in_end->next; + } + /* 'in_end' now points to the last pbuf from 'in' */ + if(in_end == priv->last) { /* this was the last pbuf in the list */ priv->first = priv->last = NULL; } else { /* pop the pbuf off the list */ - priv->first = priv->first->next; + priv->first = in_end->next; LWIP_ASSERT("should not be null since first != last!", priv->first != NULL); } } SYS_ARCH_UNPROTECT(lev); if(in != NULL) { - if(in->next != NULL) { + if(in_end->next != NULL) { /* De-queue the pbuf from its successors on the 'priv' list. */ - in->next = NULL; - /* This is built on the assumption that PBUF_RAM pbufs are in one piece! */ - LWIP_ASSERT("packet must not consist of multiple pbufs!", in->len == in->tot_len); + in_end->next = NULL; } if(netif->input(in, netif) != ERR_OK) { pbuf_free(in); } /* Don't reference the packet any more! */ in = NULL; + in_end = NULL; } /* go on while there is a packet on the list */ } while(priv->first != NULL); @@ -121,6 +126,7 @@ loopif_output(struct netif *netif, struct pbuf *p, #if !LWIP_LOOPIF_MULTITHREADING SYS_ARCH_DECL_PROTECT(lev); struct loopif_private *priv; + struct pbuf *last; #endif /* LWIP_LOOPIF_MULTITHREADING */ struct pbuf *r; err_t err; @@ -153,16 +159,16 @@ loopif_output(struct netif *netif, struct pbuf *p, through calling loopif_poll(). */ priv = (struct loopif_private*)netif->state; - /* This is built on the assumption that PBUF_RAM pbufs are in one piece! */ - LWIP_ASSERT("packet must not consist of multiple pbufs!", r->len == r->tot_len); - + /* let last point to the last pbuf in chain r */ + for (last = r; last->next != NULL; last = last->next); SYS_ARCH_PROTECT(lev); if(priv->first != NULL) { LWIP_ASSERT("if first != NULL, last must also be != NULL", priv->last != NULL); priv->last->next = r; - priv->last = r; + priv->last = last; } else { - priv->first = priv->last = r; + priv->first = r; + priv->last = last; } SYS_ARCH_UNPROTECT(lev); #endif /* LWIP_LOOPIF_MULTITHREADING */