Bug Fixes

Steve G <[email protected]> Sat, 7 Feb 2004 10:38:11 -0800 (PST)
Newsgroups gmane.network.zeroconf.workers
Message-ID <[email protected]>
Hello,

I updated to the latest cvs and looked through the code. I found
several problems. The attached patch fixes:

 - A memory leak on SIGHUP. rr_free needed to free the domain
string.

 - getifaddr can return records where ifnow->ifa_addr  is NULL.
This must be skipped or it will core dump tmdns.

 - Off by 1 in loop initing multicast sockets.

 - struct ifreq needed to be zero'd before setting the socket
option.

The patch also identifies a couple variables that seem to be
unused. It labels them with a FIXME. Either they can be deleted
or they were intended to be used, but not implemented yet.

After applying this patch, I am able to startup tmdns on the open
internet without crashing. Not that you want to do this. :)

The spec file is mostly done, I will send it in a separate
e-mail.

-Steve Grubb

__________________________________
Do you Yahoo!?
Yahoo! Finance: Get your refund fast by filing online.
http://taxes.yahoo.com/filing.html
tmdns-patch8.txt (text/plain, 6.3 KB)
diff -ur tmdns/server/conf.c tmdns.b/server/conf.c
--- tmdns/server/conf.c	2004-02-07 09:11:48.000000000 -0500
+++ tmdns.b/server/conf.c	2004-02-07 13:07:31.000000000 -0500
@@ -299,7 +299,8 @@
 		return 0;
 	 }
   } else {
-	 strcpy(config.config_file, conf_file);
+	if (config.config_file != conf_file)
+	      strcpy(config.config_file, conf_file);
   }
   while (fgets(line, 1024 , fp)) {
 	 if (!(line[0]=='#')) {	/* skip lines with comment */
@@ -551,9 +552,6 @@
 
   char ** cfname = NULL;
 
-  if( config.ex_interfaces == NULL) 
-    return 0;
-
   for( cfname = config.ex_interfaces; *cfname != NULL; cfname ++ ) {
     if( strcmp( *cfname , ifname ) == 0 ) {
       return 1;
diff -ur tmdns/server/conf.h tmdns.b/server/conf.h
--- tmdns/server/conf.h	2004-02-07 09:11:48.000000000 -0500
+++ tmdns.b/server/conf.h	2004-02-07 09:53:30.000000000 -0500
@@ -28,7 +28,6 @@
 struct config {
   int  port;
   int  daemon_mode;
-  int  no_fork;
   char service_file[CONF_PATH_LEN];
   char config_file[CONF_PATH_LEN];
   char debug_file[CONF_PATH_LEN];
diff -ur tmdns/server/info.c tmdns.b/server/info.c
--- tmdns/server/info.c	2004-02-07 09:11:50.000000000 -0500
+++ tmdns.b/server/info.c	2004-02-07 13:02:08.000000000 -0500
@@ -101,6 +101,8 @@
  *
  *****************************************************************************/
 static void free_rr(dns_rr * rr) {
+  if (rr->domain)
+    free(rr->domain);
   free(rr);
 }
 
@@ -479,6 +481,11 @@
 
     for(ifnow = ifs; ifnow; ifnow = ifnow->ifa_next) {
 	if( ifnow->ifa_flags & IFF_LOOPBACK ) continue;
+	if (ifnow->ifa_addr == NULL)
+	{
+		debug("getifaddr returned an interface with NULL address");
+		continue;
+	}
 
 	if( is_excluded_interface( ifnow->ifa_name ) ) {
 	    debug("exclude address from interface %s\n" , ifnow->ifa_name );
diff -ur tmdns/server/serv_udp.c tmdns.b/server/serv_udp.c
--- tmdns/server/serv_udp.c	2004-02-07 09:11:51.000000000 -0500
+++ tmdns.b/server/serv_udp.c	2004-02-07 12:35:13.000000000 -0500
@@ -96,7 +96,7 @@
     /* count addresses and addresses on multicast capable interfaces */
     for(ifnow = interfaces; ifnow; ifnow = ifnow->ifa_next) {
       if( ! (ifnow->ifa_flags & IFF_MULTICAST) ) n_mcast ++;
-      n_addresses ++;
+      n_addresses ++;   // FIXME: This is never used - delete?
     }
 
     /*
@@ -147,6 +147,9 @@
 	if( is_excluded_interface( ifnow->ifa_name ) ) 
 	    continue;
 
+	if (ifnow->ifa_addr == NULL)
+		continue;
+
 	switch( ifnow->ifa_addr->sa_family ) {
 	    case AF_INET:
 		{
@@ -200,7 +203,7 @@
     if( n_mcast > 0 ) {
       int i = 0;
       unsigned int mcastidx = 0;
-      char * seen = "";
+      const char * seen = "";
 
       mcast_sockets = (int *)malloc( (n_mcast+1) * sizeof(*mcast_sockets));
       if( mcast_sockets == NULL ) {
@@ -209,7 +212,7 @@
 	  return sockidx;
       }
 
-      for( i = 0; i <= n_mcast + 1 ; i ++ )  mcast_sockets[i] = -1;
+      for( i = 0; i < n_mcast + 1 ; i ++ )  mcast_sockets[i] = -1;
 
       for(ifnow = interfaces; ifnow; ifnow = ifnow->ifa_next) {
 
@@ -218,6 +221,9 @@
 	    continue;
 	}
 
+	if (ifnow->ifa_addr == NULL)
+		continue;
+
         if( (ifnow->ifa_addr->sa_family == AF_INET) && 
 	    (strcmp(ifnow->ifa_name,seen) == 0 ) )
 	{
@@ -328,6 +334,7 @@
 
   if( ifname != NULL ) {
       struct ifreq interface;
+      memset (&interface, 0, sizeof (struct ifreq));
       strncpy(interface.ifr_ifrn.ifrn_name, ifname, IFNAMSIZ);
       if( setsockopt(fd  , SOL_SOCKET, SO_BINDTODEVICE, &interface, sizeof(interface)) < 0 ) {  
           debug_perror("can not bind to device"); 
@@ -389,7 +396,7 @@
   msg.msg_iovlen     = 1;
   msg.msg_control    = (void *) adata;
   msg.msg_controllen = sizeof(adata);
-  iov.iov_base       = &(udp_pkt->buf);
+  iov.iov_base       = udp_pkt->buf;
   iov.iov_len        = sizeof(udp_pkt->buf);
 
 
diff -ur tmdns/server/tmdns.c tmdns.b/server/tmdns.c
--- tmdns/server/tmdns.c	2004-02-07 09:11:52.000000000 -0500
+++ tmdns.b/server/tmdns.c	2004-02-07 10:44:49.000000000 -0500
@@ -31,9 +31,9 @@
 static int  store_query(int sock, struct udp_packet * udp_pkt);
 static int  answer_outstanding_queries(void); 
 static void fill_outstanding_queries(struct udp_packet * udp_pkt); 
-static int announce();
-static int godbye();
-static int probe();
+static int announce(void);
+static int goodbye(void);
+static int probe(void);
 
 static volatile int go_down = 0;
 static volatile int do_config = 0;
@@ -507,7 +507,7 @@
      } /* no error */
   } /* while not go down */
 
-  godbye();
+  goodbye();
   
   unlink( config.pid_file );
   syslog(LOG_INFO,"normal exit");
@@ -685,7 +685,7 @@
  * The packet contains questions for all of out RR's plus all
  * the RR's in the authority section. 
  *****************************************************************************/
-static int probe() {
+static int probe(void) {
     dns_t answer;
     search_state s;
 
@@ -725,7 +725,7 @@
  * An announcement is simply an answer to all of our RR's
  *
  *****************************************************************************/
-static int announce() {
+static int announce(void) {
     dns_t answer;
     search_state s;
 
@@ -755,16 +755,16 @@
 }
 
 /*****************************************************************************
- * Send godbye packet to the multicast address.
+ * Send goodbye packet to the multicast address.
  *
- * Like an announcement thos is simply an answer to all of our RR's but
+ * Like an announcement that is simply an answer to all of our RR's but
  * with TTL set to 0. 
  *
  * We actually set the ttl in each RR to 0, so we need to re-init the
  * resource db afterwards.
  *
  *****************************************************************************/
-static int godbye() {
+static int goodbye(void) {
     dns_t answer;
     search_state s;
 
@@ -778,7 +778,7 @@
     while( info_search(&s) >0 ) {
 	s.data->ttl = 0;
         dns_add_rr( &answer, s.data );
-	syslog(LOG_INFO,"godbye for name\"%s\", type %d\n" , s.data->domain , s.data->type );
+	syslog(LOG_INFO,"goodbye for name\"%s\", type %d\n" , s.data->domain , s.data->type );
     }
 
     /* this is an answer */
@@ -917,7 +917,7 @@
 	info_init_search(&s,query->question[n].query_arg,query->question[n].query_type);
 
 	while( info_search(&s) >0 ) {
-	    answer_count ++;
+	    answer_count ++;	// FIXME: This is not used - delete?
 	    dns_add_rr_ttl( answer, s.data , config.default_unicast_ttl );
 	}
     }