* [ECOS] 'id' rollover bug in dns.c
@ 2009-10-01 23:11 Will Lentz
2009-10-01 23:37 ` Jay Foster
0 siblings, 1 reply; 6+ messages in thread
From: Will Lentz @ 2009-10-01 23:11 UTC (permalink / raw)
To: ecos-discuss
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] 'id' rollover bug in dns.c
2009-10-01 23:11 [ECOS] 'id' rollover bug in dns.c Will Lentz
@ 2009-10-01 23:37 ` Jay Foster
2009-10-01 23:53 ` Will Lentz
2009-10-02 0:15 ` Will Lentz
0 siblings, 2 replies; 6+ messages in thread
From: Jay Foster @ 2009-10-01 23:37 UTC (permalink / raw)
To: Will Lentz; +Cc: ecos-discuss
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [ECOS] 'id' rollover bug in dns.c
2009-10-01 23:37 ` Jay Foster
@ 2009-10-01 23:53 ` Will Lentz
2009-10-02 0:15 ` Will Lentz
1 sibling, 0 replies; 6+ messages in thread
From: Will Lentz @ 2009-10-01 23:53 UTC (permalink / raw)
To: Jay Foster; +Cc: ecos-discuss
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [ECOS] 'id' rollover bug in dns.c
2009-10-01 23:37 ` Jay Foster
2009-10-01 23:53 ` Will Lentz
@ 2009-10-02 0:15 ` Will Lentz
2009-10-02 1:11 ` Jay Foster
1 sibling, 1 reply; 6+ messages in thread
From: Will Lentz @ 2009-10-02 0:15 UTC (permalink / raw)
To: Jay Foster; +Cc: ecos-discuss
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] 'id' rollover bug in dns.c
2009-10-02 0:15 ` Will Lentz
@ 2009-10-02 1:11 ` Jay Foster
2009-10-05 15:50 ` Will Lentz
0 siblings, 1 reply; 6+ messages in thread
From: Jay Foster @ 2009-10-02 1:11 UTC (permalink / raw)
To: Will Lentz; +Cc: ecos-discuss
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [ECOS] 'id' rollover bug in dns.c
2009-10-02 1:11 ` Jay Foster
@ 2009-10-05 15:50 ` Will Lentz
0 siblings, 0 replies; 6+ messages in thread
From: Will Lentz @ 2009-10-05 15:50 UTC (permalink / raw)
To: ecos-discuss; +Cc: Jay Foster
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-10-05 15:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-01 23:11 [ECOS] 'id' rollover bug in dns.c Will Lentz
2009-10-01 23:37 ` Jay Foster
2009-10-01 23:53 ` Will Lentz
2009-10-02 0:15 ` Will Lentz
2009-10-02 1:11 ` Jay Foster
2009-10-05 15:50 ` Will Lentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).