Hi,
There's a race condition in
packages/net/ns/dns/current/include/dns_impl.inl and dns.c.
Here's a quick example of how the current code may cause a problem:
1) Call cyg_dns_res_start() in thread #1.
Assume the connect() call blocks for a few seconds.
Note that at this point 's' is valid, but 'ptdindex' in
uninitialized or invalid.
2) While thread #1 is blocked, call gethostbyname() (or
gethostbyaddr()) in thread #2.
The call to free_stored_hent() assumes 'ptdindex' is valid, but it
is not.
If you have asserts on, cyg_thread_get_data(ptdindex) will hit an
assert or return
a bogus pointer value that may get passed to free().
The attached fix:
- puts 's' and free_stored_hent()/ptdindex inside the mutex lock so
they are
always consistent with each other.
- makes 'init' file-visible so it can protect against accessing an
uninitialized mutex.
Thanks,
Will
diff -ru orig/include/dns_impl.inl new/include/dns_impl.inl
--- orig/include/dns_impl.inl 2009-07-14 15:17:05.000000000 -0700
+++ new/include/dns_impl.inl 2009-07-14 15:25:52.000000000 -0700
@@ -382,15 +382,18 @@
CYG_REPORT_FUNCNAMETYPE( "gethostbyaddr", "returning %08x" );
CYG_REPORT_FUNCARG3( "addr=%08x, len=%d, type=%d", addr, len, type );
- if ( !addr || 0 == len) {
+ if ( !init || !addr || 0 == len) {
CYG_REPORT_RETVAL( NULL );
return NULL;
}
CYG_CHECK_DATA_PTR( addr, "addr is not a valid pointer!" );
+ cyg_drv_mutex_lock(&dns_mutex);
+
/* Has the socket to the DNS server been opened? */
if (s < 0) {
+ cyg_drv_mutex_unlock(&dns_mutex);
CYG_REPORT_RETVAL( NULL );
return NULL;
}
@@ -401,12 +404,11 @@
/* Only IPv4 addresses accepted */
if ((type != AF_INET) || (len != sizeof(struct in_addr))) {
+ cyg_drv_mutex_unlock(&dns_mutex);
CYG_REPORT_RETVAL( NULL );
return NULL;
}
- cyg_drv_mutex_lock(&dns_mutex);
-
/* Build the 'hostname' we want to lookup. */
sprintf(hostname, "%d.%d.%d.%d.IN-ADDR.ARPA.",
(unsigned char)addr[3],
@@ -479,15 +481,18 @@
CYG_REPORT_FUNCNAMETYPE( "gethostbyname", "returning %08x" );
CYG_REPORT_FUNCARG1( "hostname=%08x", hostname );
- if ( !hostname ) {
+ if ( !init || !hostname ) {
CYG_REPORT_RETVAL( NULL );
return NULL;
}
CYG_CHECK_DATA_PTR( hostname, "hostname is not a valid pointer!" );
+ cyg_drv_mutex_lock(&dns_mutex);
+
/* Has the socket to the DNS server been opened? */
if (s < 0) {
+ cyg_drv_mutex_unlock(&dns_mutex);
CYG_REPORT_RETVAL( NULL );
return NULL;
}
@@ -500,6 +505,7 @@
/* It could be a dot address */
if ((hent = dot_hostname(hostname)) != NULL) {
store_hent(hent);
+ cyg_drv_mutex_unlock(&dns_mutex);
CYG_REPORT_RETVAL( hent );
return hent;
}
@@ -507,14 +513,15 @@
/* It could be a colon seperated IPv6 address */
if ((hent = colon_hostname(hostname)) != NULL) {
store_hent(hent);
+ cyg_drv_mutex_unlock(&dns_mutex);
CYG_REPORT_RETVAL( hent );
return hent;
}
#endif
CYG_REPORT_RETVAL( hent );
+ cyg_drv_mutex_unlock(&dns_mutex);
return hent;
}
- cyg_drv_mutex_lock(&dns_mutex);
if (domainname) {
if ((strlen(hostname) + strlen(domainname)) > 254) {
diff -ru orig/src/dns.c new/src/dns.c
--- orig/src/dns.c 2009-07-14 15:16:53.000000000 -0700
+++ new/src/dns.c 2009-07-14 15:24:58.000000000 -0700
@@ -79,6 +79,7 @@
#include <cyg/ns/dns/dns_priv.h>
+static int init =0; /* Is DNS initialized? */
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 */
@@ -262,7 +263,6 @@
int cyg_dns_res_start(char * dns_server) {
- static int init =0;
struct addrinfo * res;
int err;
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
