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

Jay Foster jay@systech.com
Fri Oct 2 01:11:00 GMT 2009


Perhaps.  My DNS code has several other changes to allow this and other 
things.  It's been a while since I've examined this so I have forgotten 
the details.
Jay

On 10/1/2009 5:15 PM, Will Lentz wrote:
> Although... 'id' is protected by locking dns_mutex, so multiple
> concurrent DNS queries shouldn't be an issue.
>
> Cheers,
> Will
>
> -----Original Message-----
> From: Will Lentz
> Sent: Thursday, October 01, 2009 4:53 PM
> To: 'Jay Foster'
> Cc: ecos-discuss@ecos.sourceware.org
> Subject: RE: [ECOS] 'id' rollover bug in dns.c
>
> 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