* Fix all uses of pbuf_header to check the return value. In some

cases just assert if it fails as I'm not sure how to fix them, but
    this is no worse than before when they would carry on regardless
    of the failure.
This commit is contained in:
kieranm 2007-03-21 12:55:00 +00:00
parent b422864d5d
commit 7294cb080b
12 changed files with 106 additions and 34 deletions

View File

@ -66,6 +66,11 @@ HISTORY
* api_lib.c: Use memcpy in netbuf_copy_partial. * api_lib.c: Use memcpy in netbuf_copy_partial.
++ Bug fixes: ++ Bug fixes:
2007-03-21 Kieran Mansley
* Fix all uses of pbuf_header to check the return value. In some
cases just assert if it fails as I'm not sure how to fix them, but
this is no worse than before when they would carry on regardless
of the failure.
2007-03-21 Kieran Mansley 2007-03-21 Kieran Mansley
* sockets.c, igmp.c, igmp.h, memp.h: Fix C++ style comments and * sockets.c, igmp.c, igmp.h, memp.h: Fix C++ style comments and

View File

@ -138,7 +138,12 @@ ethernet_input(struct pbuf *p, struct netif *netif)
etharp_ip_input( netif, p); etharp_ip_input( netif, p);
#endif #endif
/* skip Ethernet header */ /* skip Ethernet header */
pbuf_header(p, (s16_t)(-sizeof(struct eth_hdr))); if(pbuf_header(p, (s16_t)(-sizeof(struct eth_hdr)))) {
LWIP_ASSERT("Can't move over header in packet", 0);
pbuf_free(p);
p = NULL;
}
else
/* pass to IP layer */ /* pass to IP layer */
ip_input(p, netif); ip_input(p, netif);
break; break;

View File

@ -111,7 +111,9 @@ icmp_input(struct pbuf *p, struct netif *inp)
/* increase number of echo replies attempted to send */ /* increase number of echo replies attempted to send */
snmp_inc_icmpoutechoreps(); snmp_inc_icmpoutechoreps();
pbuf_header(p, hlen); if(pbuf_header(p, hlen))
LWIP_ASSERT("Can't move over header in packet", 0);
else
ip_output_if(p, &(iphdr->src), IP_HDRINCL, ip_output_if(p, &(iphdr->src), IP_HDRINCL,
IPH_TTL(iphdr), 0, IP_PROTO_ICMP, inp); IPH_TTL(iphdr), 0, IP_PROTO_ICMP, inp);
break; break;

View File

@ -220,8 +220,10 @@ ip_input(struct pbuf *p, struct netif *inp) {
LWIP_DEBUGF("ip_input: p->len %"U16_F" p->tot_len %"U16_F"\n", p->len, p->tot_len);*/ LWIP_DEBUGF("ip_input: p->len %"U16_F" p->tot_len %"U16_F"\n", p->len, p->tot_len);*/
#endif /* IP_DEBUG */ #endif /* IP_DEBUG */
if(pbuf_header(p, -IP_HLEN)) {
pbuf_header(p, -IP_HLEN); LWIP_ASSERT("Can't move over header in packet", 0);
return;
}
switch (iphdr->nexthdr) { switch (iphdr->nexthdr) {
case IP_PROTO_UDP: case IP_PROTO_UDP:
@ -266,7 +268,7 @@ ip_output_if (struct pbuf *p, struct ip_addr *src, struct ip_addr *dest,
PERF_START; PERF_START;
printf("len %"U16_F" tot_len %"U16_F"\n", p->len, p->tot_len); LWIP_DEBUGF(IP_DEBUG, ("len %"U16_F" tot_len %"U16_F"\n", p->len, p->tot_len));
if (pbuf_header(p, IP_HLEN)) { if (pbuf_header(p, IP_HLEN)) {
LWIP_DEBUGF(IP_DEBUG, ("ip_output: not enough room for IP header in pbuf\n")); LWIP_DEBUGF(IP_DEBUG, ("ip_output: not enough room for IP header in pbuf\n"));
#ifdef IP_STATS #ifdef IP_STATS
@ -275,13 +277,13 @@ ip_output_if (struct pbuf *p, struct ip_addr *src, struct ip_addr *dest,
return ERR_BUF; return ERR_BUF;
} }
printf("len %"U16_F" tot_len %"U16_F"\n", p->len, p->tot_len); LWIP_DEBUGF(IP_DEBUG, ("len %"U16_F" tot_len %"U16_F"\n", p->len, p->tot_len));
iphdr = p->payload; iphdr = p->payload;
if (dest != IP_HDRINCL) { if (dest != IP_HDRINCL) {
printf("!IP_HDRLINCL\n"); LWIP_DEBUGF(IP_DEBUG, ("!IP_HDRLINCL\n"));
iphdr->hoplim = ttl; iphdr->hoplim = ttl;
iphdr->nexthdr = proto; iphdr->nexthdr = proto;
iphdr->len = htons(p->tot_len - IP_HLEN); iphdr->len = htons(p->tot_len - IP_HLEN);

View File

@ -482,12 +482,15 @@ pbuf_header(struct pbuf *p, s16_t header_size_increment)
LWIP_ASSERT("increment_magnitude <= p->len", increment_magnitude <= p->len); LWIP_ASSERT("increment_magnitude <= p->len", increment_magnitude <= p->len);
} else { } else {
increment_magnitude = header_size_increment; increment_magnitude = header_size_increment;
#if 0 /* Can't assert these as some callers speculatively call
pbuf_header() to see if it's OK. Will return 1 below instead. */
/* Check that we've got the correct type of pbuf to work with */ /* Check that we've got the correct type of pbuf to work with */
LWIP_ASSERT("p->flags == PBUF_FLAG_RAM || p->flags == PBUF_FLAG_POOL", LWIP_ASSERT("p->flags == PBUF_FLAG_RAM || p->flags == PBUF_FLAG_POOL",
p->flags == PBUF_FLAG_RAM || p->flags == PBUF_FLAG_POOL); p->flags == PBUF_FLAG_RAM || p->flags == PBUF_FLAG_POOL);
/* Check that we aren't going to move off the beginning of the pbuf */ /* Check that we aren't going to move off the beginning of the pbuf */
LWIP_ASSERT("p->payload - increment_magnitude >= p + sizeof(struct pbuf)", LWIP_ASSERT("p->payload - increment_magnitude >= p + sizeof(struct pbuf)",
(u8_t *)p->payload - increment_magnitude >= (u8_t *)p + sizeof(struct pbuf)); (u8_t *)p->payload - increment_magnitude >= (u8_t *)p + sizeof(struct pbuf));
#endif
} }
flags = p->flags; flags = p->flags;
@ -520,6 +523,11 @@ pbuf_header(struct pbuf *p, s16_t header_size_increment)
return 1; return 1;
} }
} }
else {
/* Unknown type */
LWIP_ASSERT("bad pbuf flag type", 0);
return 1;
}
/* modify pbuf length fields */ /* modify pbuf length fields */
p->len += header_size_increment; p->len += header_size_increment;
p->tot_len += header_size_increment; p->tot_len += header_size_increment;

View File

@ -220,7 +220,10 @@ raw_sendto(struct raw_pcb *pcb, struct pbuf *p, struct ip_addr *ipaddr)
} else { } else {
/* first pbuf q equals given pbuf */ /* first pbuf q equals given pbuf */
q = p; q = p;
pbuf_header(q, -IP_HLEN); if(pbuf_header(q, -IP_HLEN)) {
LWIP_ASSERT("Can't restore header we just removed!", 0);
return ERR_MEM;
}
} }
if ((netif = ip_route(ipaddr)) == NULL) { if ((netif = ip_route(ipaddr)) == NULL) {

View File

@ -803,7 +803,10 @@ snmp_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, struct ip_addr *addr,
/* suppress unused argument warning */ /* suppress unused argument warning */
if (arg); if (arg);
/* peek in the UDP header (goto IP payload) */ /* peek in the UDP header (goto IP payload) */
pbuf_header(p, UDP_HLEN); if(pbuf_header(p, UDP_HLEN)){
LWIP_ASSERT("Can't move to UDP header", 0);
return;
}
udphdr = p->payload; udphdr = p->payload;
/* check if datagram is really directed at us (including broadcast requests) */ /* check if datagram is really directed at us (including broadcast requests) */

View File

@ -149,7 +149,14 @@ tcp_input(struct pbuf *p, struct netif *inp)
/* Move the payload pointer in the pbuf so that it points to the /* Move the payload pointer in the pbuf so that it points to the
TCP data instead of the TCP header. */ TCP data instead of the TCP header. */
hdrlen = TCPH_HDRLEN(tcphdr); hdrlen = TCPH_HDRLEN(tcphdr);
pbuf_header(p, -(hdrlen * 4)); if(pbuf_header(p, -(hdrlen * 4))){
/* drop short packets */
LWIP_DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet\n"));
TCP_STATS_INC(tcp.lenerr);
TCP_STATS_INC(tcp.drop);
pbuf_free(p);
return;
}
/* Convert fields in TCP header to host byte order. */ /* Convert fields in TCP header to host byte order. */
tcphdr->src = ntohs(tcphdr->src); tcphdr->src = ntohs(tcphdr->src);
@ -900,9 +907,13 @@ tcp_receive(struct tcp_pcb *pcb)
p->len = 0; p->len = 0;
p = p->next; p = p->next;
} }
pbuf_header(p, -off); if(pbuf_header(p, -off))
/* Do we need to cope with this failing? Assert for now */
LWIP_ASSERT("pbuf_header failed", 0);
} else { } else {
pbuf_header(inseg.p, -off); if(pbuf_header(inseg.p, -off))
/* Do we need to cope with this failing? Assert for now */
LWIP_ASSERT("pbuf_header failed", 0);
} }
/* KJM following line changed to use p->payload rather than inseg->p->payload /* KJM following line changed to use p->payload rather than inseg->p->payload
to fix bug #9076 */ to fix bug #9076 */

View File

@ -312,7 +312,12 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
/* fit within max seg size */ /* fit within max seg size */
useg->len + queue->len <= pcb->mss) { useg->len + queue->len <= pcb->mss) {
/* Remove TCP header from first segment of our to-be-queued list */ /* Remove TCP header from first segment of our to-be-queued list */
pbuf_header(queue->p, -TCP_HLEN); if(pbuf_header(queue->p, -TCP_HLEN)) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
TCP_STATS_INC(tcp.err);
goto memerr;
}
pbuf_cat(useg->p, queue->p); pbuf_cat(useg->p, queue->p);
useg->len += queue->len; useg->len += queue->len;
useg->next = queue->next; useg->next = queue->next;

View File

@ -96,7 +96,7 @@ udp_input(struct pbuf *p, struct netif *inp)
iphdr = p->payload; iphdr = p->payload;
if (p->tot_len < (IPH_HL(iphdr) * 4 + UDP_HLEN)) { if (p->tot_len < (IPH_HL(iphdr) * 4 + UDP_HLEN) || pbuf_header(p, -(s16_t)(IPH_HL(iphdr) * 4))) {
/* drop short packets */ /* drop short packets */
LWIP_DEBUGF(UDP_DEBUG, LWIP_DEBUGF(UDP_DEBUG,
("udp_input: short UDP datagram (%"U16_F" bytes) discarded\n", p->tot_len)); ("udp_input: short UDP datagram (%"U16_F" bytes) discarded\n", p->tot_len));
@ -107,8 +107,6 @@ udp_input(struct pbuf *p, struct netif *inp)
goto end; goto end;
} }
pbuf_header(p, -(s16_t)(IPH_HL(iphdr) * 4));
udphdr = (struct udp_hdr *)p->payload; udphdr = (struct udp_hdr *)p->payload;
LWIP_DEBUGF(UDP_DEBUG, ("udp_input: received datagram of length %"U16_F"\n", p->tot_len)); LWIP_DEBUGF(UDP_DEBUG, ("udp_input: received datagram of length %"U16_F"\n", p->tot_len));
@ -205,7 +203,14 @@ udp_input(struct pbuf *p, struct netif *inp)
} }
#endif #endif
} }
pbuf_header(p, -UDP_HLEN); if(pbuf_header(p, -UDP_HLEN)) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
UDP_STATS_INC(udp.drop);
snmp_inc_udpinerrors();
pbuf_free(p);
goto end;
}
if (pcb != NULL) { if (pcb != NULL) {
snmp_inc_udpindatagrams(); snmp_inc_udpindatagrams();
/* callback */ /* callback */
@ -319,7 +324,7 @@ udp_send(struct udp_pcb *pcb, struct pbuf *p)
} }
/* not enough space to add an UDP header to first pbuf in given p chain? */ /* not enough space to add an UDP header to first pbuf in given p chain? */
if (p->flags == PBUF_FLAG_ROM || p->flags == PBUF_FLAG_REF || pbuf_header(p, UDP_HLEN)) { if (pbuf_header(p, UDP_HLEN)) {
/* allocate header in a seperate new pbuf */ /* allocate header in a seperate new pbuf */
q = pbuf_alloc(PBUF_IP, UDP_HLEN, PBUF_RAM); q = pbuf_alloc(PBUF_IP, UDP_HLEN, PBUF_RAM);
/* new header pbuf could not be allocated? */ /* new header pbuf could not be allocated? */

View File

@ -1296,7 +1296,11 @@ static void pppInput(void *arg)
pd = ((struct pppInputHeader *)nb->payload)->unit; pd = ((struct pppInputHeader *)nb->payload)->unit;
protocol = ((struct pppInputHeader *)nb->payload)->proto; protocol = ((struct pppInputHeader *)nb->payload)->proto;
pbuf_header(nb, -(int)sizeof(struct pppInputHeader)); if(pbuf_header(nb, -(int)sizeof(struct pppInputHeader))) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
return;
}
#if LINK_STATS #if LINK_STATS
lwip_stats.link.recv++; lwip_stats.link.recv++;
@ -1383,7 +1387,11 @@ static void pppInput(void *arg)
/* No handler for this protocol so reject the packet. */ /* No handler for this protocol so reject the packet. */
PPPDEBUG((LOG_INFO, "pppInput[%d]: rejecting unsupported proto 0x%04X len=%d\n", pd, protocol, nb->len)); PPPDEBUG((LOG_INFO, "pppInput[%d]: rejecting unsupported proto 0x%04X len=%d\n", pd, protocol, nb->len));
pbuf_header(nb, sizeof(protocol)); if (pbuf_header(nb, sizeof(protocol))) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
goto drop;
}
#if BYTE_ORDER == LITTLE_ENDIAN #if BYTE_ORDER == LITTLE_ENDIAN
protocol = htons(protocol); protocol = htons(protocol);
memcpy(nb->payload, &protocol, sizeof(protocol)); memcpy(nb->payload, &protocol, sizeof(protocol));

View File

@ -371,13 +371,19 @@ u_int vj_compress_tcp(
if (!comp->compressSlot || comp->last_xmit != cs->cs_id) { if (!comp->compressSlot || comp->last_xmit != cs->cs_id) {
comp->last_xmit = cs->cs_id; comp->last_xmit = cs->cs_id;
hlen -= deltaS + 4; hlen -= deltaS + 4;
pbuf_header(pb, -hlen); if(pbuf_header(pb, -hlen)){
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
}
cp = (u_char *)pb->payload; cp = (u_char *)pb->payload;
*cp++ = changes | NEW_C; *cp++ = changes | NEW_C;
*cp++ = cs->cs_id; *cp++ = cs->cs_id;
} else { } else {
hlen -= deltaS + 3; hlen -= deltaS + 3;
pbuf_header(pb, -hlen); if(pbuf_header(pb, -hlen)) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
}
cp = (u_char *)pb->payload; cp = (u_char *)pb->payload;
*cp++ = changes; *cp++ = changes;
} }
@ -573,7 +579,12 @@ int vj_uncompress_tcp(
cs->cs_ip.ip_sum = (u_short)(~tmp); cs->cs_ip.ip_sum = (u_short)(~tmp);
/* Remove the compressed header and prepend the uncompressed header. */ /* Remove the compressed header and prepend the uncompressed header. */
pbuf_header(n0, -vjlen); if(pbuf_header(n0, -vjlen)) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
goto bad;
}
if(MEM_ALIGN(n0->payload) != n0->payload) { if(MEM_ALIGN(n0->payload) != n0->payload) {
struct pbuf *np, *q; struct pbuf *np, *q;
@ -586,7 +597,11 @@ int vj_uncompress_tcp(
goto bad; goto bad;
} }
pbuf_header(np, -cs->cs_hlen); if(pbuf_header(np, -cs->cs_hlen)) {
/* Can we cope with this failing? Just assert for now */
LWIP_ASSERT("pbuf_header failed\n", 0);
goto bad;
}
bufptr = n0->payload; bufptr = n0->payload;
for(q = np; q != NULL; q = q->next) { for(q = np; q != NULL; q = q->next) {