This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
RE: 'id' rollover bug in dns.c
I entered the following bug so hopefully either fix will get into CVS:
https://bugzilla.ecoscentric.com/show_bug.cgi?id=1000835
-----Original Message-----
From: Jay Foster [mailto:jay@systech.com]
Sent: Thursday, October 01, 2009 6:12 PM
To: Will Lentz
Cc: ecos-discuss@ecos.sourceware.org
Subject: Re: [ECOS] 'id' rollover bug in dns.c
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