[ECOS] 'id' rollover bug in dns.c

Will Lentz Will_Lentz@Trimble.com
Thu Oct 1 23:53:00 GMT 2009


Thanks!  That's a better solution.

Will 

-----Original Message-----
From: Jay Foster [mailto:jay@systech.com] 
Sent: Thursday, October 01, 2009 4:38 PM
To: Will Lentz
Cc: ecos-discuss@ecos.sourceware.org
Subject: Re: [ECOS] 'id' rollover bug in dns.c

I discovered that issue some time ago and resolved it in a different 
manner that also allows multiple concurrent DNS queries to be sent from 
different threads.  The solution was to change the declaration of 'id' 
to be unsigned to match the definition of id in the dns_header
structure.

-static short id = 0;
+static unsigned short id = 0;

Then declare a new variable in send_recv()

     unsigned short req_id;

Then assign this variable with the ID value contained in the DNS request

     dns_hdr = (struct dns_header *) &msg_buf[0];
+    req_id = ((struct dns_header *)msg)->id;
     do {
         /* Send a request to each configured DNS server */

Then compare the ID in the responses with req_id

                 if (dns_hdr->id != req_id) {
                     continue;
                 }

This avoids ignoring valid DNS responses when the global variable, id, 
is incremented by another concurrent DNS request.

Jay

On 10/1/2009 4:11 PM, Will Lentz wrote:
> Hi,
>
> There is a rollover bug in the current CVS copy of dns.c. Around line
> 237 there is the following code:
>              /* Reply to an old query. Ignore it */
>              if (ntohs(dns_hdr->id) != (id-1)) {
>
> If dns_hdr->id == 5 and id == 6, then the 'if' condition is false as
> expected.
>
> If dns_hdr->id == 0xFFFF and id == 0, then the 'if' condition is
> incorrectly true.
>
> The simple patch below fixes the issue:
> --- dns_old.c   2009-10-01 16:01:41.000000000 -0700
> +++ dns.c       2009-10-01 16:01:45.000000000 -0700
> @@ -234,7 +234,7 @@
>               }
>
>               /* Reply to an old query. Ignore it */
> -            if (ntohs(dns_hdr->id) != (id-1)) {
> +            if (ntohs(dns_hdr->id) != (cyg_uint16)(id-1)) {
>                   continue;
>               }
>               finished = true;
>
>
> Cheers,
> Will
>
>    

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss



More information about the Ecos-discuss mailing list