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

[Jay Foster <jay at systech dot com>]

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