This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: A bug in DNS lookup.
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: Frank Huang <fhuang at airdefense dot net>
- Cc: "ecos-discuss at ecos dot sourceware dot org" <ecos-discuss at ecos dot sourceware dot org>, eCos Patches List <ecos-patches at ecos dot sourceware dot org>
- Date: Tue, 12 Aug 2008 17:51:38 +0100
- Subject: Re: [ECOS] A bug in DNS lookup.
- Openpgp: id=A5FB74E6
- References: <D59016254F27D245B40D4CEECE1BDF600B1781F8C7@shake2.airdefense.net>
Frank Huang wrote:
> Hi,
>
> I found a "signed and unsigned" bug in eCos DNS lookup code. If you guys
> agree that, please fix it and put it in the 3.0 release.
>
> In dns_impl.inl, there is a function build_query() which build the DNS
> query packet. It uses the following line code to set the transaction ID.
>
>
> dns_hdr->id = htons(id++);
>
> The type of dns_hdr->id is a unsigned 16 bit, but the id in dns.c is a
> short integer.
dns_hdr->id is in fact an unsigned int, but in a 16-bit bitfield. This is
different from being a natural 16-bit quantity like an unsigned short, and
is I think the real cause of the problem.
> According to the protocol, this transaction ID will be
> increased frequently, so when the id increased from 0x7fff to 0x8000, it
> corrupts the next element's data which is a flag. The flag indicates the
> type of the packet. It should be indicated as "standard query" but it
> becomes to "standard query response" when it hits the bug.
>
> I force my system keep doing DNS lookup, it hits the bug in about 1 hour
> with about 32000 lookup.
>
> My fixing is that set the id in dns.c to unsigned short integer. The
> path of the files I am talking about is under eocs/packages/net/ns/dns/.
Thanks, for the report. The attached patch includes your suggestion, and
just to be explicit, includes a cast to a cyg_uint16.
Jifl
--
eCosCentric Limited http://www.eCosCentric.com/ The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK. Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------ Opinions==mine
Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/ns/dns/current/ChangeLog,v
retrieving revision 1.21
diff -u -5 -p -r1.21 ChangeLog
--- ChangeLog 19 May 2006 10:14:33 -0000 1.21
+++ ChangeLog 12 Aug 2008 16:51:28 -0000
@@ -1,5 +1,12 @@
+2008-08-12 Jonathan Larmour <jifl@eCosCentric.com>
+
+ * src/dns.c: id global should be unsigned, in line with DNS header.
+ * include/dns_impl.inl (build_query): Guarantee id in header is only
+ 16-bits.
+ Thanks to Frank Huang for the report.
+
2006-05-19 Andrew Lunn <andrew.lunn@ascom.ch>
* tests/dns1.c (dns_test_thread): Use CYG_NELEM from infra.
2005-07-29 Andrew Lunn <andrew.lunn@ascom.ch>
Index: include/dns_impl.inl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/ns/dns/current/include/dns_impl.inl,v
retrieving revision 1.7
diff -u -5 -p -r1.7 dns_impl.inl
--- include/dns_impl.inl 30 Jul 2005 11:26:29 -0000 1.7
+++ include/dns_impl.inl 12 Aug 2008 16:51:28 -0000
@@ -197,11 +197,11 @@ build_query(const unsigned char * msg, c
unsigned char *ptr;
int len;
/* Fill out the header */
dns_hdr = (struct dns_header *) msg;
- dns_hdr->id = htons(id++);
+ dns_hdr->id = (cyg_uint16)htons(id++);
dns_hdr->rd = true;
dns_hdr->opcode = DNS_QUERY;
dns_hdr->qdcount = htons(1);
/* Now the question we want to ask */
Index: src/dns.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/ns/dns/current/src/dns.c,v
retrieving revision 1.10
diff -u -5 -p -r1.10 dns.c
--- src/dns.c 30 Jul 2005 11:26:29 -0000 1.10
+++ src/dns.c 12 Aug 2008 16:51:28 -0000
@@ -78,11 +78,11 @@
#include <string.h>
#include <stdlib.h>
#include <cyg/ns/dns/dns_priv.h>
-static short id = 0; /* ID of the last query */
+static cyg_uint16 id = 0; /* ID of the last query */
static int s = -1; /* Socket to the DNS server */
static cyg_drv_mutex_t dns_mutex; /* Mutex to stop multiple queries as once */
static cyg_ucount32 ptdindex; /* Index for the per thread data */
static char * domainname=NULL; /* Domain name used for queries */
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss