dns: fix rr->srv memory leak in rrcopy, mark rr->sig->signer, dn aging, cleanup

4 files changed, 66 insertions, 105 deletions

diff --git a/sys/src/cmd/ndb/dn.c b/sys/src/cmd/ndb/dn.c
index 367edacb7..6c8df3dc0 100644
--- a/sys/src/cmd/ndb/dn.c
+++ b/sys/src/cmd/ndb/dn.c
@@ -14,14 +14,14 @@
  * figure it out.
  */
 enum {
-	Deftarget	= 1<<30,	/* effectively disable aging */
-	Minage		= 1<<30,
-	Defagefreq	= 1<<30,	/* age names this often (seconds) */
+//	Deftarget	= 1<<30,	/* effectively disable aging */
+//	Minage		= 1<<30,
+//	Defagefreq	= 1<<30,	/* age names this often (seconds) */
 
 	/* these settings will trigger frequent aging */
-//	Deftarget	= 4000,
-//	Minage		=  5*60,
-//	Defagefreq	= 15*60,	/* age names this often (seconds) */
+	Deftarget	= 4000,
+	Minage		=  5*60,
+	Defagefreq	= 15*60,	/* age names this often (seconds) */
 };
 
 /*
@@ -217,7 +217,6 @@ dnlookup(char *name, int class, int enter)
 	dp = emalloc(sizeof(*dp));
 	dp->magic = DNmagic;
 	dp->name = estrdup(name);
-	assert(dp->name != nil);
 	dp->class = class;
 	dp->rr = 0;
 	dp->referenced = now;
@@ -446,6 +445,9 @@ dnagenever(DN *dp, int dolock)
 			MARK(rp->host);
 			MARK(rp->rmb);
 			break;
+		case Tsig:
+			MARK(rp->sig->signer);
+			break;
 		}
 	}
 
@@ -564,6 +566,9 @@ dnageall(int doit)
 					REF(rp->host);
 					REF(rp->rmb);
 					break;
+				case Tsig:
+					REF(rp->sig->signer);
+					break;
 				}
 			}
 
@@ -571,7 +576,7 @@ dnageall(int doit)
 	for(i = 0; i < HTLEN; i++){
 		l = &ht[i];
 		for(dp = *l; dp; dp = *l){
-			if(dp->rr == 0 && dp->refs == 0 && !dp->keep){
+			if(dp->rr == 0 && dp->refs == 0 && dp->keep == 0){
 				assert(dp->magic == DNmagic);
 				*l = dp->next;
 
@@ -714,9 +719,12 @@ putactivity(int recursive)
 	}
 	unlock(&dnvars);
 
+	dncheck(0, 1);
+
 	db2cache(needrefresh);
 	dnageall(0);
 
+	dncheck(0, 1);
 	/* let others back in */
 	lastclean = now;
 	needrefresh = 0;
@@ -751,8 +759,6 @@ rrattach1(RR *new, int auth)
 	DN *dp;
 
 	assert(new->magic == RRmagic && !new->cached);
-
-//	dnslog("rrattach1: %s", new->owner->name);
 	if(!new->db) {
 		/*
 		 * try not to let responses expire before we
@@ -763,7 +769,7 @@ rrattach1(RR *new, int auth)
 	} else
 		new->expire = now + Year;
 	dp = new->owner;
-	assert(dp->magic == DNmagic);
+	assert(dp != nil && dp->magic == DNmagic);
 	new->auth |= auth;
 	new->next = 0;
 
@@ -847,7 +853,7 @@ rrattach1(RR *new, int auth)
 void
 rrattach(RR *rp, int auth)
 {
-	RR *next, *tp;
+	RR *next;
 	DN *dp;
 
 	lock(&dnlock);
@@ -855,25 +861,13 @@ rrattach(RR *rp, int auth)
 		next = rp->next;
 		rp->next = nil;
 		dp = rp->owner;
-
-//		dnslog("rrattach: %s", rp->owner->name);
 		/* avoid any outside spoofing; leave keepers alone */
-		if(cfg.cachedb && !rp->db && inmyarea(rp->owner->name)
+		if(cfg.cachedb && !rp->db && inmyarea(dp->name)
 //		    || dp->keep			/* TODO: make this work */
 		    )
 			rrfree(rp);
-		else {
-			/* ameliorate the memory leak (someday delete this) */
-			if (0 && rrlistlen(dp->rr) > 50 && !dp->keep) {
-				dnslog("rrattach(%s): rr list too long; "
-					"freeing it", dp->name);
-				tp = dp->rr;
-				dp->rr = nil;
-				rrfreelist(tp);
-			} else
-				USED(dp);
+		else
 			rrattach1(rp, auth);
-		}
 	}
 	unlock(&dnlock);
 }
@@ -882,42 +876,32 @@ rrattach(RR *rp, int auth)
 RR**
 rrcopy(RR *rp, RR **last)
 {
-	Cert *cert;
-	Key *key;
-	Null *null;
 	RR *nrp;
 	SOA *soa;
+	Srv *srv;
+	Key *key;
+	Cert *cert;
 	Sig *sig;
+	Null *null;
 	Txt *t, *nt, **l;
 
+	assert(rp->magic == RRmagic);
 	if (canlock(&dnlock))
 		abort();	/* rrcopy called with dnlock not held */
 	nrp = rralloc(rp->type);
-	setmalloctag(nrp, getcallerpc(&rp));
 	switch(rp->type){
-	case Ttxt:
-		*nrp = *rp;
-		l = &nrp->txt;
-		*l = nil;
-		for(t = rp->txt; t != nil; t = t->next){
-			nt = emalloc(sizeof(*nt));
-			nt->p = estrdup(t->p);
-			nt->next = nil;
-			*l = nt;
-			l = &nt->next;
-		}
-		break;
 	case Tsoa:
 		soa = nrp->soa;
 		*nrp = *rp;
 		nrp->soa = soa;
-		*nrp->soa = *rp->soa;
-		nrp->soa->slaves = copyserverlist(rp->soa->slaves);
+		*soa = *rp->soa;
+		soa->slaves = copyserverlist(rp->soa->slaves);
 		break;
 	case Tsrv:
+		srv = nrp->srv;
 		*nrp = *rp;
-		nrp->srv = emalloc(sizeof *nrp->srv);
-		*nrp->srv = *rp->srv;
+		nrp->srv = srv;
+		*srv = *rp->srv;
 		break;
 	case Tkey:
 		key = nrp->key;
@@ -927,14 +911,6 @@ rrcopy(RR *rp, RR **last)
 		key->data = emalloc(key->dlen);
 		memmove(key->data, rp->key->data, rp->key->dlen);
 		break;
-	case Tsig:
-		sig = nrp->sig;
-		*nrp = *rp;
-		nrp->sig = sig;
-		*sig = *rp->sig;
-		sig->data = emalloc(sig->dlen);
-		memmove(sig->data, rp->sig->data, rp->sig->dlen);
-		break;
 	case Tcert:
 		cert = nrp->cert;
 		*nrp = *rp;
@@ -943,6 +919,14 @@ rrcopy(RR *rp, RR **last)
 		cert->data = emalloc(cert->dlen);
 		memmove(cert->data, rp->cert->data, rp->cert->dlen);
 		break;
+	case Tsig:
+		sig = nrp->sig;
+		*nrp = *rp;
+		nrp->sig = sig;
+		*sig = *rp->sig;
+		sig->data = emalloc(sig->dlen);
+		memmove(sig->data, rp->sig->data, rp->sig->dlen);
+		break;
 	case Tnull:
 		null = nrp->null;
 		*nrp = *rp;
@@ -951,10 +935,24 @@ rrcopy(RR *rp, RR **last)
 		null->data = emalloc(null->dlen);
 		memmove(null->data, rp->null->data, rp->null->dlen);
 		break;
+	case Ttxt:
+		*nrp = *rp;
+		l = &nrp->txt;
+		*l = nil;
+		for(t = rp->txt; t != nil; t = t->next){
+			nt = emalloc(sizeof(*nt));
+			nt->p = estrdup(t->p);
+			nt->next = nil;
+			*l = nt;
+			l = &nt->next;
+		}
+		break;
 	default:
 		*nrp = *rp;
 		break;
 	}
+	nrp->pc = getcallerpc(&rp);
+	setmalloctag(nrp, nrp->pc);
 	nrp->cached = 0;
 	nrp->next = 0;
 	*last = nrp;
@@ -1231,8 +1229,6 @@ rrfmt(Fmt *f)
 		fmtprint(&fstr, "\t%s", dnname(rp->ip));
 		break;
 	case Tptr:
-//		fmtprint(&fstr, "\t%s(%lud)", dnname(rp->ptr),
-//			rp->ptr? rp->ptr->ordinal: "<null>");
 		fmtprint(&fstr, "\t%s", dnname(rp->ptr));
 		break;
 	case Tsoa:
@@ -1506,7 +1502,7 @@ slave(Request *req)
 
 	/* limit parallelism */
 	procs = getactivity(req, 1);
-	if (procs > stats.slavehiwat)
+	if(procs > stats.slavehiwat)
 		stats.slavehiwat = procs;
 	if(procs > Maxactive){
 		if(traceactivity)
@@ -2003,8 +1999,7 @@ rrfree(RR *rp)
 	RR *nrp;
 	Txt *t;
 
-	assert(rp->magic == RRmagic);
-	assert(!rp->cached);
+	assert(rp->magic == RRmagic && !rp->cached);
 
 	dp = rp->owner;
 	if(dp){
@@ -2044,8 +2039,7 @@ rrfree(RR *rp)
 		free(rp->null);
 		break;
 	case Ttxt:
-		while(rp->txt != nil){
-			t = rp->txt;
+		while(t = rp->txt){
 			rp->txt = t->next;
 			free(t->p);
 			memset(t, 0, sizeof *t);	/* cause trouble */
diff --git a/sys/src/cmd/ndb/dnresolve.c b/sys/src/cmd/ndb/dnresolve.c
index 68d24137b..67b060b71 100644
--- a/sys/src/cmd/ndb/dnresolve.c
+++ b/sys/src/cmd/ndb/dnresolve.c
@@ -587,6 +587,7 @@ udpport(char *mtpt)
 void
 initdnsmsg(DNSmsg *mp, RR *rp, int flags, ushort reqno)
 {
+	memset(mp, 0, sizeof *mp);
 	mp->flags = flags;
 	mp->id = reqno;
 	mp->qd = rp;
@@ -594,16 +595,6 @@ initdnsmsg(DNSmsg *mp, RR *rp, int flags, ushort reqno)
 		mp->qdcount = 1;
 }
 
-DNSmsg *
-newdnsmsg(RR *rp, int flags, ushort reqno)
-{
-	DNSmsg *mp;
-
-	mp = emalloc(sizeof *mp);
-	initdnsmsg(mp, rp, flags, reqno);
-	return mp;
-}
-
 /* generate a DNS UDP query packet */
 int
 mkreq(DN *dp, int type, uchar *buf, int flags, ushort reqno)
@@ -620,7 +611,6 @@ mkreq(DN *dp, int type, uchar *buf, int flags, ushort reqno)
 	/* make request and convert it to output format */
 	rp = rralloc(type);
 	rp->owner = dp;
-	memset(&m, 0, sizeof m);
 	initdnsmsg(&m, rp, flags, reqno);
 	len = convDNS2M(&m, &buf[Udphdrsize], Maxudp);
 	rrfreelist(rp);
diff --git a/sys/src/cmd/ndb/dns.c b/sys/src/cmd/ndb/dns.c
index c0e814b95..e21c3ab88 100644
--- a/sys/src/cmd/ndb/dns.c
+++ b/sys/src/cmd/ndb/dns.c
@@ -101,8 +101,7 @@ void	rwstat(Job*, Mfile*);
 void	sendmsg(Job*, char*);
 void	setext(char*, int, char*);
 
-static char *lookupqueryold(Job*, Mfile*, Request*, char*, char*, int, int);
-static char *lookupquerynew(Job*, Mfile*, Request*, char*, char*, int, int);
+static char *lookupquery(Job*, Mfile*, Request*, char*, char*, int, int);
 static char *respond(Job*, Mfile*, RR*, char*, int, int);
 
 void
@@ -731,6 +730,8 @@ rwrite(Job *job, Mfile *mf, Request *req)
 		dndump("/lib/ndb/dnsdump2");
 	} else if(strcmp(job->request.data, "debug")==0)
 		debug ^= 1;
+	else if(strcmp(job->request.data, "testing")==0)
+		testing ^= 1;
 	else if(strcmp(job->request.data, "dump")==0)
 		dndump("/lib/ndb/dnsdump");
 	else if(strcmp(job->request.data, "poolcheck")==0)
@@ -803,7 +804,7 @@ rwrite(Job *job, Mfile *mf, Request *req)
 	} else
 		wantsav = 0;
 
-	err = lookupqueryold(job, mf, req, errbuf, p, wantsav, rooted);
+	err = lookupquery(job, mf, req, errbuf, p, wantsav, rooted);
 send:
 	dncheck(0, 1);
 	job->reply.count = cnt;
@@ -821,7 +822,7 @@ send:
  * but here we just call dnresolve directly.
  */
 static char *
-lookupqueryold(Job *job, Mfile *mf, Request *req, char *errbuf, char *p,
+lookupquery(Job *job, Mfile *mf, Request *req, char *errbuf, char *p,
 	int wantsav, int rooted)
 {
 	int status;
@@ -878,35 +879,12 @@ respond(Job *job, Mfile *mf, RR *rp, char *errbuf, int status, int wantsav)
 		mf->rr[mf->nrr] = n;
 	}
 	unlock(&joblock);
-	rrfreelist(rp);
-	return nil;
-}
-
-/* simulate what dnsudpserver does */
-static char *
-lookupquerynew(Job *job, Mfile *mf, Request *req, char *errbuf, char *p,
-	int wantsav, int)
-{
-	char *err;
-	uchar buf[Udphdrsize + Maxudp + 1024];
-	DNSmsg *mp;
-	DNSmsg repmsg;
-	RR *rp;
 
-	dncheck(0, 1);
-
-	memset(&repmsg, 0, sizeof repmsg);
-	rp = rralloc(mf->type);
-	rp->owner = dnlookup(p, Cin, 1);
-	mp = newdnsmsg(rp, Frecurse|Oquery, (ushort)rand());
-
-	dnserver(mp, &repmsg, req, buf, Rok);
+	lock(&dnlock);
+	rrfreelist(rp);
+	unlock(&dnlock);
 
-	freeanswers(mp);
-	err = respond(job, mf, repmsg.an, errbuf, Rok, wantsav);
-	repmsg.an = nil;		/* freed above */
-	freeanswers(&repmsg);
-	return err;
+	return nil;
 }
 
 void
diff --git a/sys/src/cmd/ndb/dns.h b/sys/src/cmd/ndb/dns.h
index fd60d122e..0b40d76e4 100644
--- a/sys/src/cmd/ndb/dns.h
+++ b/sys/src/cmd/ndb/dns.h
@@ -301,12 +301,12 @@ struct RR
 	};
 	union {			/* discriminated by type */
 		SOA	*soa;	/* soa timers - soa */
+		Srv	*srv;
 		Key	*key;
 		Cert	*cert;
 		Sig	*sig;
 		Null	*null;
 		Txt	*txt;
-		Srv	*srv;
 	};
 };
 
@@ -533,7 +533,6 @@ int	udpport(char *);
 int	mkreq(DN *dp, int type, uchar *buf, int flags, ushort reqno);
 int	seerootns(void);
 void	initdnsmsg(DNSmsg *mp, RR *rp, int flags, ushort reqno);
-DNSmsg*	newdnsmsg(RR *rp, int flags, ushort reqno);
 
 /* dnserver.c */
 void	dnserver(DNSmsg*, DNSmsg*, Request*, uchar *, int);