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