Hi,
I'm feeling my way into the kannel source, and I think I've identified
a race condition in the gw_gethostbyname() function in gwlib/protected.c
In particular, the gethostbyname function call returns some static data.
I've rolled patch to fix this, noting that:
1. The only callers in the of gw_gethostbyname in tree currently are in
gwlib/socket.c
2. Those callers only use the hostent.h_name and hostent.h_addr_list[0]
fields in the struct hostent.
3. I've been careful to free the memory allocated as close to the call
to gw_gethostbyname() as possible.
The patch thus copies mallocs space for these fields, and copies the
relevant data, and then frees it.
I know that the BSDs don't support gethostbyname_r() function call (and
that it's a pig to make portable, and isn't part of the standards). Is
there a reason Kannel uses gethostbyname in preference to getaddrinfo()?
Regards
Kieran
diff -ur -x '*.o' cvs/gateway/gwlib/protected.c work/gateway/gwlib/protected.c
--- cvs/gateway/gwlib/protected.c 2000-10-23 18:56:05.000000000 +0100
+++ work/gateway/gwlib/protected.c 2002-08-21 04:00:55.000000000 +0100
@@ -92,19 +92,69 @@
return ret;
}
+/* This function:
+ 1. Mallocs enough memory to copy relevent fields of src
+ 2. Copies those fields into dest
+ This is done because the struct that gethostbyname returns contains pointers
+ to static data.
+ Note: since kannel only uses the hostent.h_name and hostent.h_addr_list[0]
+ elements of the returned hostent currently, that's all that is copied.
+*/
+void malloc_and_copy_hostent (struct hostent * dest, const struct hostent * src) {
+ int count;
+ char *cp;
+ /* Mallocs and copies hostent.h_name element */
+ count = strlen (src->h_name) + 1;
+ dest->h_name = (char *)gw_native_malloc (count);
+ memcpy (dest->h_name, src->h_name, count);
+
+ /* Mallocs an array of size 2 for hostent.h_addr_list */
+ count = sizeof (src->h_addr_list);
+ dest->h_addr_list = (char **)gw_native_malloc (2 * sizeof(char *));
+
+ /* Mallocs and copies first element of src->h_addr_list[] */
+ count = strlen (src->h_addr_list[0]) + 1;
+ cp = (char *)gw_native_malloc (count);
+ memcpy(cp, src->h_addr_list[0], count);
+
+ /* Sets h_addr_list[0] to copy, and [1] to NULL */
+ dest->h_addr_list[0] = cp;
+ dest->h_addr_list[1] = NULL;
+
+ return;
+}
+
+/* This function frees allocated hostent structures, as malloced by
+ malloc_and_copy_hostent() defined above. This function needs to
+ be called every time gw_gethostbyname() is called to avoid memory
+ leaks
+*/
+void free_hostent(struct hostent * in) {
+ int i;
+ gw_free (in->h_name);
+ for (i=0; in->h_addr_list[i] != NULL; i++) {
+ gw_free (in->h_addr_list[i]);
+ }
+ gw_free (in->h_addr_list);
+}
+
+/* Pre: The pointer "ent" must point to a valid struct hostent
+ Post: if the return value == 0, fields in ent point to malloced
+ memory which should be freed using free_hostent()
+*/
int gw_gethostbyname(struct hostent *ent, const char *name)
{
int ret;
struct hostent *p;
lock(GETHOSTBYNAME);
- p = gethostbyname(name);
+ p = gethostbyname(name);
if (p == NULL)
ret = -1;
else {
ret = 0;
- *ent = *p;
+ malloc_and_copy_hostent(ent, p);
}
unlock(GETHOSTBYNAME);
return ret;
diff -ur -x '*.o' cvs/gateway/gwlib/protected.h work/gateway/gwlib/protected.h
--- cvs/gateway/gwlib/protected.h 2001-01-29 13:01:43.000000000 +0000
+++ work/gateway/gwlib/protected.h 2002-08-21 03:36:35.000000000 +0100
@@ -21,6 +21,7 @@
struct tm gw_gmtime(time_t t);
int gw_rand(void);
int gw_gethostbyname(struct hostent *ret, const char *name);
+void free_hostent(struct hostent *);
/*
* Make it harder to use these by mistake.
diff -ur -x '*.o' cvs/gateway/gwlib/socket.c work/gateway/gwlib/socket.c
--- cvs/gateway/gwlib/socket.c 2002-05-02 15:44:54.000000000 +0100
+++ work/gateway/gwlib/socket.c 2002-08-21 04:02:29.000000000 +0100
@@ -58,6 +58,7 @@
goto error;
}
addr.sin_addr = *(struct in_addr *) hostinfo.h_addr;
+ free_hostent(&hostinfo);
}
reuse = 1;
@@ -116,7 +117,8 @@
addr.sin_family = AF_INET;
addr.sin_port = htons(port);
addr.sin_addr = *(struct in_addr *) hostinfo.h_addr;
-
+ free_hostent(&hostinfo);
+
if (our_port > 0 || (interface_name != NULL && strcmp(interface_name, "*") != 0))
{
int reuse;
@@ -131,6 +133,7 @@
goto error;
}
o_addr.sin_addr = *(struct in_addr *) o_hostinfo.h_addr;
+ free_hostent(&o_hostinfo);
}
reuse = 1;
@@ -305,6 +308,7 @@
return -1;
}
sa.sin_addr = *(struct in_addr *) hostinfo.h_addr;
+ free_hostent(&hostinfo);
}
if (bind(s, (struct sockaddr *) &sa, (int) sizeof(sa)) == -1) {
@@ -335,6 +339,7 @@
return NULL;
}
sa.sin_addr = *(struct in_addr *) h.h_addr_list[0];
+ free_hostent(&h);
}
return octstr_create_from_data((char *) &sa, sizeof(sa));
@@ -440,6 +445,7 @@
} else {
official_name = octstr_create(h.h_name);
official_ip = gw_netaddr_to_octstr(AF_INET, h.h_addr);
+ free_hostent(&h);
}
}
