Hello,
I have been studying these valgrind errors as well, and finally found
the problem. Actually, there are three problems.
1. In ares_query.c , in find_query_by_id we compare q->qid (which is a
short int variable) with qid, which is declared as an int variable.
Moreover, DNS_HEADER_SET_QID is used to set the value of qid, but
DNS_HEADER_SET_QID sets only the first two bytes of qid. I think that
qid should be declared as "unsigned short" in this function.
2. The same problem occurs in ares_process.c, process_answer() .
query->qid (an unsigned short integer variable) is compared with id,
which is an integer variable. Moreover, id is initialized from
DNS_HEADER_QID which sets only the first two bytes of id. I think that
the id variable should be declared as "unsigned short" in this function.
Even after declaring these variables as "unsigned short", the valgrind
errors are still there. Which brings us to the third problem.
3. The third problem is that Valgrind assumes that query->qid is not
initialised correctly. And it does that because query->qid is set from
DNS_HEADER_QID(qbuf); Valgrind says that qbuf has unitialised bytes. And
qbuf has uninitialised bytes because of channel->next_id . And next_id
is set by ares_init.c:ares__generate_new_id() . I found that putting
short r=0 in this function (instead of short r) makes all Valgrind
warnings go away.
I have studied ares__rc4() too, and this is the offending line:
buffer_ptr[counter] ^= state[xorIndex]; (ares_query.c:62)
This is what triggers Valgrind.. buffer_ptr is unitialised in this
function, and by applying ^= on it, it remains unitialised.
I have attached a quick patch that solves the Valgrind issues (although,
maybe short r = 0 should be short r = random_number() for security
issues).
-- Best regards, Vlad DINULESCU AVIRA - Software developer vlad.dinulescu_at_avira.com http://www.avira.com
diff -urN c-ares-1.4.0/ares_init.c c-ares-patched/ares_init.c
--- c-ares-1.4.0/ares_init.c 2007-06-04 10:36:31.000000000 +0300
+++ c-ares-patched/ares_init.c 2007-07-09 18:23:30.000000000 +0300
@@ -1338,7 +1338,7 @@
short ares__generate_new_id(rc4_key* key)
{
- short r;
+ short r=0;
ares__rc4(key, (unsigned char *)&r, sizeof(r));
return r;
}
diff -urN c-ares-1.4.0/ares_process.c c-ares-patched/ares_process.c
--- c-ares-1.4.0/ares_process.c 2007-06-04 10:36:31.000000000 +0300
+++ c-ares-patched/ares_process.c 2007-07-09 18:24:05.000000000 +0300
@@ -400,7 +400,8 @@
static void process_answer(ares_channel channel, unsigned char *abuf,
int alen, int whichserver, int tcp, time_t now)
{
- int id, tc, rcode;
+ int tc, rcode;
+ unsigned short id;
struct query *query;
/* If there's no room in the answer for a header, we can't do much
diff -urN c-ares-1.4.0/ares_query.c c-ares-patched/ares_query.c
--- c-ares-1.4.0/ares_query.c 2007-06-08 11:43:14.000000000 +0300
+++ c-ares-patched/ares_query.c 2007-07-09 18:23:50.000000000 +0300
@@ -67,7 +67,7 @@
static struct query* find_query_by_id(ares_channel channel, int id)
{
- int qid;
+ unsigned short qid;
struct query* q;
DNS_HEADER_SET_QID(((unsigned char*)&qid), id);