nsd users - Jul 2012 - [PATCH] nsd-patch: fix segfault after renaming slave zone

Hi all,
we have discovered a segfault in nsd-patch when renaming slave zone in nsd 
config file if some data for this zone still exists in the IXFR diff 
database.
In my case, the zone "black" was renamed to
"blackinwhite":
> root at ggd115:/cage/nsd/var/nsd/zones#nsd-patch -c
> /cage/nsd/etc/nsd-dns-slave.conf
> reading database 
> reading updates to database
> [1343043191] nsd-patch[10800]: error: xfr: zone black. not in config.
> [1343043191] nsd-patch[10800]: error: no zone exists
> writing changed zones
> Segmentation fault (core dumped)
The problem is that on line 407 of nsd-patch it tries to printf() a 
message "zone %s had not changed", where %s is
zone->opts->name:
>                 for(zone = db->zones; zone; zone = zone->next)
>                 {
>                         if(!force_write && !zone->updated) {
>                                 fprintf(stdout, "zone %s had not
> changed.\n", zone->opts->name);
>                                 continue;
>                         }
zone->opts is filled in in difffile.c around line 675:
> zone->opts = zone_options_find(opt, domain_dname(zone->apex));
>         if(!zone->opts) {
>                 log_msg(LOG_ERR, "xfr: zone %s not in config.",
>                         dname_to_string(zone_name,0));
>                 return 0;
>         }
As a result, nsd-patch tries to dereference a null pointer when trying to 
print zone name.
I think the proper fix is to move the code that adds zone structure to the 
linked list at the very end of find_zone(). Attached patch fixes the issue 
described above. This patch is for nsd 3.2.11.

Please review and comment if you find it nessesary/useful/awful :-)
--
Best regards,
Ilya Bakulin

genua
Gesellschaft fuer Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
-------------- next part --------------
A non-text attachment was scrubbed...
Name: difffile.c.diff
Type: text/x-diff
Size: 826 bytes
Desc: not available
URL:
<http://lists.nlnetlabs.nl/pipermail/nsd-users/attachments/20120723/ebf8fa84/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL:
<http://lists.nlnetlabs.nl/pipermail/nsd-users/attachments/20120723/ebf8fa84/attachment-0001.bin>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Ilya,

Thanks for your report. Considering your question:
> Please review and comment if you find it nessesary/useful/awful
> :-)
We think it is useful :-)

I have applied a similar fix in the NSD 3.2 branch (r3617). Instead of
moving down the code that adds the zone structure to the list, we
moved the lookup zone in options to above. This way, if the zone is
not in the options, we don't even have to allocate memory for it.

Best regards,
  Matthijs

Index: difffile.c
==================================================================- ---
difffile.c	(revision 3615)
+++ difffile.c	(working copy)
@@ -636,6 +636,7 @@
 {
 	domain_type *domain;
 	zone_type* zone;
+	zone_options_t* opts;
 	domain = domain_table_find(db->domains, zone_name);
 	if(!domain) {
 		DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfr: creating domain %s",
@@ -654,6 +655,13 @@
 			return zone;
 		}
 	}
+	/* lookup in config */
+	opts = zone_options_find(opt, domain_dname(domain));
+	if(!opts) {
+		log_msg(LOG_ERR, "xfr: zone %s not in config.",
+			dname_to_string(zone_name,0));
+		return 0;
+	}
 	/* create the zone */
 	DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfr: creating zone_type %s",
 		dname_to_string(zone_name,0)));
@@ -663,6 +671,7 @@
 		exit(1);
 	}
 	zone->next = db->zones;
+	zone->opts = opts;
 	db->zones = zone;
 	db->zone_count++;
 	zone->apex = domain;
@@ -679,12 +688,6 @@
 		exit(1);
 	}
 	memset(zone->dirty, 0, sizeof(uint8_t)*child_count);
- -	zone->opts = zone_options_find(opt, domain_dname(zone->apex));
- -	if(!zone->opts) {
- -		log_msg(LOG_ERR, "xfr: zone %s not in config.",
- -			dname_to_string(zone_name,0));
- -		return 0;
- -	}
 #ifdef NSEC3
 #ifndef FULL_PREHASH
 	zone->nsec3_domains = NULL;



On 07/23/2012 01:58 PM, Ilya Bakulin wrote:
> Hi all, we have discovered a segfault in nsd-patch when renaming
> slave zone in nsd config file if some data for this zone still
> exists in the IXFR diff database. In my case, the zone "black"
was
> renamed to "blackinwhite":
>> root at ggd115:/cage/nsd/var/nsd/zones#nsd-patch -c 
>> /cage/nsd/etc/nsd-dns-slave.conf reading database reading updates
>> to database [1343043191] nsd-patch[10800]: error: xfr: zone
>> black. not in config. [1343043191] nsd-patch[10800]: error: no
>> zone exists writing changed zones Segmentation fault (core
>> dumped)
> 
> The problem is that on line 407 of nsd-patch it tries to printf() a
>  message "zone %s had not changed", where %s is
zone->opts->name:
>> for(zone = db->zones; zone; zone = zone->next) { if(!force_write
>> && !zone->updated) { fprintf(stdout, "zone %s had not 
>> changed.\n", zone->opts->name); continue; }
> 
> zone->opts is filled in in difffile.c around line 675:
>> zone->opts = zone_options_find(opt, domain_dname(zone->apex)); 
>> if(!zone->opts) { log_msg(LOG_ERR, "xfr: zone %s not in
>> config.", dname_to_string(zone_name,0)); return 0; }
> 
> As a result, nsd-patch tries to dereference a null pointer when
> trying to print zone name. I think the proper fix is to move the
> code that adds zone structure to the linked list at the very end of
> find_zone(). Attached patch fixes the issue described above. This
> patch is for nsd 3.2.11.
> 
> Please review and comment if you find it nessesary/useful/awful
> :-) -- Best regards, Ilya Bakulin
> 
> genua Gesellschaft fuer Netzwerk- und Unix-Administration mbH 
> Domagkstrasse 7, 85551 Kirchheim bei Muenchen tel +49 89 991950-0,
> fax -999, www.genua.de Geschaeftsfuehrer: Dr. Magnus Harlander, Dr.
> Michaela Harlander, Bernhard Schneck. Amtsgericht Muenchen HRB
> 98238
> 
> 
> 
> _______________________________________________ nsd-users mailing
> list nsd-users at NLnetLabs.nl 
> http://open.nlnetlabs.nl/mailman/listinfo/nsd-users
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJQDU90AAoJEA8yVCPsQCW5lyUIALWU/mqtluhg/l2rsXqduvA4
sl2rPAtgnYcM3EhNzNbM+COFAS7atFRWDPhC4sQadEImOoMjRbCxmyuU6yCo2THZ
QlBVazcqB5wWN/vCAnbPfLNUKH4M3rMTVuGqRACHHdEy2WvW41LMBEbXu8Goapc+
bB6Mu0Akh2XP1nZ3apfQYoVjkX07bymVPi04PUytILV4pY7L9ZTfWA0cgsYFzh0W
5qSdYecF0TXAG5R/swr0mz05IqVOWSYG9opiOEZNl1cB8u1L75sXqCuvIct7Lyve
lmNEhbbwO8oMKc8Xx7qHOVlWqyciIUtEl2UD2c2ggm3aVQRC/lmWnUum9p83V1o=l8va
-----END PGP SIGNATURE-----