On Tue, Jan 20, 2015 at 05:27:11PM -0600, Brent Cook wrote:
> On Mon, Jan 12, 2015 at 10:02 AM, Todd C. Miller
> <[email protected]> wrote:
> > On Mon, 12 Jan 2015 09:12:02 -0600, Brent Cook wrote:
> >
> >> - Nothing seems to free the result of host_dns(), so add
> >> host_dns_free() and call after each query.
> >> - If imsg_add fails, it frees buf. Avoid dereferencing the freed buf
> >> afterward in imsg_close().
> >
> > That looks good to me.
> >
> > - todd
>
> Unfortunately, this caused a regression:
>
> 1. bring down networking
> 2. start ntpd
> 3. start networking
>
> Result is, it doesn't properly resolve the hosts on the next poll.
>
> I just got finished bisecting to point to this as the problem diff.
> Will be back online in about 30 minutes when I'm near a power adapter
> again.
After some head-scratching, I found the root cause. Whether or not
host_dns succeeds, we always need to close/send the HOST_DNS imsg.
ok?
Index: ntp_dns.c
===================================================================
RCS file: /cvs/src/usr.sbin/ntpd/ntp_dns.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 ntp_dns.c
--- ntp_dns.c 13 Jan 2015 02:28:56 -0000 1.7
+++ ntp_dns.c 21 Jan 2015 01:30:04 -0000
@@ -167,12 +167,12 @@ dns_dispatch_imsg(void)
buf = NULL;
break;
}
- if (buf)
- imsg_close(ibuf_dns, buf);
}
host_dns_free(hn);
hn = NULL;
}
+ if (buf)
+ imsg_close(ibuf_dns, buf);
break;
default:
break;
Index: ntpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
retrieving revision 1.87
diff -u -p -u -p -r1.87 ntpd.c
--- ntpd.c 19 Jan 2015 20:47:03 -0000 1.87
+++ ntpd.c 21 Jan 2015 01:30:04 -0000
@@ -366,12 +366,12 @@ dispatch_imsg(struct ntpd_conf *lconf)
buf = NULL;
break;
}
- if (buf)
- imsg_close(ibuf, buf);
}
host_dns_free(hn);
hn = NULL;
}
+ if (buf)
+ imsg_close(ibuf, buf);
break;
default:
break;
