added some defensive programming to nmbd. This mostly means zeroing

areas of memory before freeing them.

While doing this I also found a couple of real bugs. In two places we
were freeing some memory that came from the stack, which leads to
a certain core dump on many sytems.
(This used to be commit c5e5c25c854e54f59291057ba47c4701b5910ebe)

diff --git a/source3/nmbd/nmbd_become_lmb.c b/source3/nmbd/nmbd_become_lmb.c
index 2d007ecd7538c674b188530bef5c008e60f7e3d7..3fe6f596fa621d505eaa0c931989fc6ad7428bcf 100644 (file)
--- a/source3/nmbd/nmbd_become_lmb.c
+++ b/source3/nmbd/nmbd_become_lmb.c
@@ -212,9 +212,9 @@ static void release_1d_name( struct subnet_record *subrec, char *workgroup_name,
   if((namerec = find_name_on_subnet( subrec, &nmbname, FIND_SELF_NAME))!=NULL)
   {
     struct userdata_struct *userdata;
+    int size = sizeof(struct userdata_struct) + sizeof(BOOL);
 
-    if((userdata = (struct userdata_struct *)malloc( 
-                      sizeof(struct userdata_struct) + sizeof(BOOL))) == NULL)
+    if((userdata = (struct userdata_struct *)malloc(size)) == NULL)
     {
       DEBUG(0,("release_1d_name: malloc fail.\n"));
       return;
@@ -230,7 +230,7 @@ static void release_1d_name( struct subnet_record *subrec, char *workgroup_name,
                  unbecome_local_master_fail,
                  userdata);
 
-    free((char *)userdata);
+    zero_free(userdata, size);
   }
 }
 
@@ -526,6 +526,7 @@ void become_local_master_browser(struct subnet_record *subrec, struct work_recor
 {
   struct server_record *servrec;
   struct userdata_struct *userdata;
+  int size = sizeof(struct userdata_struct) + sizeof(fstring) + 1;
 
   /* Sanity check. */
   if (!lp_local_master())
@@ -561,7 +562,7 @@ in workgroup %s on subnet %s\n",
   subrec->work_changed = True;
 
   /* Setup the userdata_struct. */
-  if((userdata = (struct userdata_struct *)malloc(sizeof(struct userdata_struct) + sizeof(fstring)+1)) == NULL)
+  if((userdata = (struct userdata_struct *)malloc(size)) == NULL)
   {
     DEBUG(0,("become_local_master_browser: malloc fail.\n"));
     return;
@@ -578,7 +579,7 @@ in workgroup %s on subnet %s\n",
                 become_local_master_fail1,
                 userdata);
 
-  free((char *)userdata);
+  zero_free(userdata, size);
 }
 
 /***************************************************************

diff --git a/source3/nmbd/nmbd_browsesync.c b/source3/nmbd/nmbd_browsesync.c
index dcf2ea3c4876658639169e171745dc9c036c23a7..bc0cf43f7c262f30b7b13034c10022eb87408b61 100644 (file)
--- a/source3/nmbd/nmbd_browsesync.c
+++ b/source3/nmbd/nmbd_browsesync.c
@@ -386,6 +386,7 @@ static void find_domain_master_name_query_success(struct subnet_record *subrec,
   struct work_record *work;
   struct nmb_name nmbname;
   struct userdata_struct *userdata;
+  int size = sizeof(struct userdata_struct) + sizeof(fstring)+1;
 
   if (!(work = find_workgroup_on_subnet(subrec, q_name->name))) {
       DEBUG(0, ("find_domain_master_name_query_success: failed to find \
@@ -418,7 +419,7 @@ workgroup %s\n", q_name->name ));
 
   /* Setup the userdata_struct - this is copied so we can use
      a stack variable for this. */
-  if((userdata = (struct userdata_struct *)malloc(sizeof(struct userdata_struct) + sizeof(fstring)+1)) == NULL)
+  if((userdata = (struct userdata_struct *)malloc(size)) == NULL)
   {
     DEBUG(0, ("find_domain_master_name_query_success: malloc fail.\n"));
     return;
@@ -434,7 +435,7 @@ workgroup %s\n", q_name->name ));
                domain_master_node_status_fail,
                userdata);
 
-  free((char *)userdata);
+  zero_free(userdata, size);
 }
 
 /****************************************************************************

diff --git a/source3/nmbd/nmbd_incomingrequests.c b/source3/nmbd/nmbd_incomingrequests.c
index a3afbe827a2b210bb7480df9d931a5cd7d293b54..c2b8be212f0987020d6d22536e5ce2ceac19dd08 100644 (file)
--- a/source3/nmbd/nmbd_incomingrequests.c
+++ b/source3/nmbd/nmbd_incomingrequests.c
@@ -576,7 +576,7 @@ on the same subnet (%s) as the requestor. Not replying.\n",
   if (!success && bcast)
   {
     if((prdata != rdata) && (prdata != NULL))
-      free(rdata);
+      free(prdata);
     return; /* Never reply with a negative response to broadcasts. */
   }
 
@@ -589,7 +589,7 @@ on the same subnet (%s) as the requestor. Not replying.\n",
   if(!success && !bcast && nmb->header.nm_flags.recursion_desired)
   {
     if((prdata != rdata) && (prdata != NULL))
-      free(rdata);
+      free(prdata);
     return;
   }
 

diff --git a/source3/nmbd/nmbd_namelistdb.c b/source3/nmbd/nmbd_namelistdb.c
index 29d822550c4c951ff357f1adf2d2fa783b75292f..de5835a11515d228b7cafb4e7a22c005df6e3160 100644 (file)
--- a/source3/nmbd/nmbd_namelistdb.c
+++ b/source3/nmbd/nmbd_namelistdb.c
@@ -111,6 +111,8 @@ void remove_name_from_namelist( struct subnet_record *subrec,
 
   if(namerec->data.ip != NULL)
     free((char *)namerec->data.ip);
+
+  ZERO_STRUCTP(namerec);
   free((char *)namerec);
 
   subrec->namelist_changed = True;
@@ -218,6 +220,8 @@ struct name_record *add_name_to_subnet( struct subnet_record *subrec,
   if( NULL == namerec->data.ip )
   {
      DEBUG( 0, ( "add_name_to_subnet: malloc fail when creating ip_flgs.\n" ) );
+
+     ZERO_STRUCTP(namerec);
      free( (char *)namerec );
      return NULL;
   }

diff --git a/source3/nmbd/nmbd_responserecordsdb.c b/source3/nmbd/nmbd_responserecordsdb.c
index 6dae0d43e9270e41883aa8309bd8e23ddf5f94e9..21defa970cdb6a2baa05d1f98bc129031ed2e8b5 100644 (file)
--- a/source3/nmbd/nmbd_responserecordsdb.c
+++ b/source3/nmbd/nmbd_responserecordsdb.c
@@ -80,16 +80,19 @@ void remove_response_record(struct subnet_record *subrec,
 
   if(rrec->userdata)
   {
-    if(rrec->userdata->free_fn)
-      (*rrec->userdata->free_fn)(rrec->userdata);
-    else
-      free((char *)rrec->userdata);
+         if(rrec->userdata->free_fn) {
+                 (*rrec->userdata->free_fn)(rrec->userdata);
+         } else {
+                 ZERO_STRUCTP(rrec->userdata);
+                 free((char *)rrec->userdata);
+         }
   }
 
   /* Ensure we can delete. */
   rrec->packet->locked = False;
   free_packet(rrec->packet);
 
+  ZERO_STRUCTP(rrec);
   free((char *)rrec);
 
   num_response_packets--; /* count of total number of packets still around */
@@ -135,6 +138,7 @@ struct response_record *make_response_record( struct subnet_record *subrec,
       if((rrec->userdata = (*userdata->copy_fn)(userdata)) == NULL)
       {
         DEBUG(0,("make_response_queue_record: copy fail for userdata.\n"));
+       ZERO_STRUCTP(rrec);
         free(rrec);
         return NULL;
       }
@@ -146,6 +150,7 @@ struct response_record *make_response_record( struct subnet_record *subrec,
            malloc(sizeof(struct userdata_struct)+userdata->userdata_len)) == NULL)
       {
         DEBUG(0,("make_response_queue_record: malloc fail for userdata.\n"));
+       ZERO_STRUCTP(rrec);
         free(rrec);
         return NULL;
       }

diff --git a/source3/nmbd/nmbd_serverlistdb.c b/source3/nmbd/nmbd_serverlistdb.c
index 6ff6ad5c5d7fb492dec595a78e84a9eec4f2663b..a4dab6f419db67e01dd72fdacc00be6df5156f41 100644 (file)
--- a/source3/nmbd/nmbd_serverlistdb.c
+++ b/source3/nmbd/nmbd_serverlistdb.c
@@ -56,6 +56,7 @@ void remove_all_servers(struct work_record *work)
     if (work->serverlist == servrec)
       work->serverlist = servrec->next;
 
+    ZERO_STRUCTP(servrec);
     free((char *)servrec);
 
   }
@@ -120,6 +121,7 @@ void remove_server_from_workgroup(struct work_record *work, struct server_record
   if (work->serverlist == servrec) 
     work->serverlist = servrec->next; 
 
+  ZERO_STRUCTP(servrec);
   free((char *)servrec);
   work->subnet->work_changed = True;
 }

diff --git a/source3/nmbd/nmbd_subnetdb.c b/source3/nmbd/nmbd_subnetdb.c
index 942175c9f82b5ff61aff498c9c11a3a4f54fddb1..edc930c2054139288e88331b5e15add9c4a36b1a 100644 (file)
--- a/source3/nmbd/nmbd_subnetdb.c
+++ b/source3/nmbd/nmbd_subnetdb.c
@@ -180,6 +180,7 @@ static struct subnet_record *make_subnet(char *name, enum subnet_type type,
     DEBUG(0,("make_subnet: malloc fail for subnet name !\n"));
     close(nmb_sock);
     close(dgram_sock);
+    ZERO_STRUCTP(subrec);
     free((char *)subrec);
     return(NULL);
   }

diff --git a/source3/nmbd/nmbd_winsproxy.c b/source3/nmbd/nmbd_winsproxy.c
index 5635124bcda9637173ad1b5af22a86c571cc1e1b..2084d3915ab64d66415c94d018d19763dd884300 100644 (file)
--- a/source3/nmbd/nmbd_winsproxy.c
+++ b/source3/nmbd/nmbd_winsproxy.c
@@ -191,6 +191,7 @@ static void wins_proxy_userdata_free_fn(struct userdata_struct *userdata)
   p->locked = False;
 
   free_packet(p);
+  ZERO_STRUCTP(userdata);
   free((char *)userdata);
 }
 

diff --git a/source3/nmbd/nmbd_workgroupdb.c b/source3/nmbd/nmbd_workgroupdb.c
index 0b0b72485473bc4b32624c94c8e1e4f1e6e9f8a7..ac25127e82c68bd455ac68b9a057e5a615b6a11a 100644 (file)
--- a/source3/nmbd/nmbd_workgroupdb.c
+++ b/source3/nmbd/nmbd_workgroupdb.c
@@ -158,6 +158,7 @@ static struct work_record *remove_workgroup_from_subnet(struct subnet_record *su
     if (subrec->workgrouplist == work)
       subrec->workgrouplist = work->next; 
   
+    ZERO_STRUCTP(work);
     free((char *)work);
   }