buglwIP - A Lightweight TCP/IP stack - Bugs: bug #41508, using NULL pointer in...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #41508: using NULL pointer in icmp_send_response

Submitted by:  luozhaosong <geliter>
Submitted on:  Sat 08 Feb 2014 03:04:35 AM UTC  
 
Category: IPv4Severity: 3 - Normal
Item Group: NoneStatus: Invalid
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.4.1

Fri 21 Feb 2014 01:33:00 AM UTC, comment #2:

I have check the code, yes, you are right. sorry for waste your time -_-|||

luozhaosong <geliter>
Thu 20 Feb 2014 08:49:12 PM UTC, comment #1:

I don't understand, did you have any problems with this? The NULL is passed via ip_output to ip_output_if_opt(), where it is checked by ip_addr_isany(src), which includes a NULL check.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 08 Feb 2014 03:04:35 AM UTC, original submission:

I am using lwip-1.4.1.
in the function icmp_send_response(\lwip-1.4.1\src\core\ipv4\icmp.c),when it invoke ip_output,it use a null pointer as the second parameter:ip_output(q, NULL, &iphdr_src, ICMP_TTL, 0, IP_PROTO_ICMP).
but in ip_output, it does not check if the second parameter is NULL.
so i think this is a bug,but i am not sure how to fix it. appreciate for any response !!

i fix it as following :
static void
icmp_send_response(struct pbuf *p, u8_t type, u8_t code)
{
struct pbuf *q;
struct ip_hdr *iphdr;
/* we can use the echo header here */
struct icmp_echo_hdr *icmphdr;
ip_addr_t iphdr_src;
ip_addr_t iphdr_dst;
/* ICMP header + IP header + 8 bytes of data */
q = pbuf_alloc(PBUF_IP, sizeof(struct icmp_echo_hdr) + IP_HLEN + ICMP_DEST_UNREACH_DATASIZE,
PBUF_RAM);
if (q == NULL) {
LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded: failed to allocate pbuf for ICMP packet.\n"));
return;
}
LWIP_ASSERT("check that first pbuf can hold icmp message",
(q->len >= (sizeof(struct icmp_echo_hdr) + IP_HLEN + ICMP_DEST_UNREACH_DATASIZE)));

iphdr = (struct ip_hdr *)p->payload;
LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded from "));
ip_addr_debug_print(ICMP_DEBUG, &(iphdr->src));
LWIP_DEBUGF(ICMP_DEBUG, (" to "));
ip_addr_debug_print(ICMP_DEBUG, &(iphdr->dest));
LWIP_DEBUGF(ICMP_DEBUG, ("\n"));

icmphdr = (struct icmp_echo_hdr *)q->payload;
icmphdr->type = type;
icmphdr->code = code;
icmphdr->id = 0;
icmphdr->seqno = 0;

/* copy fields from original packet */
SMEMCPY((u8_t )q->payload + sizeof(struct icmp_echo_hdr), (u8_t )p->payload,
IP_HLEN + ICMP_DEST_UNREACH_DATASIZE);

/* calculate checksum */
icmphdr->chksum = 0;
icmphdr->chksum = inet_chksum(icmphdr, q->len);
ICMP_STATS_INC(icmp.xmit);
/* increase number of messages attempted to send */
snmp_inc_icmpoutmsgs();
/* increase number of destination unreachable messages attempted to send */
snmp_inc_icmpouttimeexcds();
ip_addr_copy(iphdr_src, iphdr->src);
ip_addr_copy(iphdr_dst, iphdr->dest);

ip_output(q, &iphdr_dst, &iphdr_src, ICMP_TTL, 0, IP_PROTO_ICMP);
pbuf_free(q);
}

luozhaosong <geliter>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by geliter (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 20 Feb 2014 08:49:12 PM UTCgoldsimonStatusNone=>Invalid
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1