OK bcook@
On Fri, Dec 2, 2016 at 10:29 AM, Rafael Zalamena <[email protected]>
wrote:
> On Sat, Oct 01, 2016 at 07:05:51PM +0200, Rafael Zalamena wrote:
> > The ntpd(8) constraint fork+exec diff changed the way the constraint
> > processes are created, but then it introduced new calloc()s to avoid
> > increasing diff size and to focus on the problem. Now that the fork+exec
> > is in, this diff make those variables to become a part of the stack.
> >
> > No functional changes, just changing variables storage location.
> >
> > ok?
>
> Ping.
>
> Updated diff to apply on the latest ntpd sources.
>
> ok?
>
> Index: usr.sbin/ntpd//constraint.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/ntpd/constraint.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 constraint.c
> --- usr.sbin/ntpd//constraint.c 18 Oct 2016 22:05:47 -0000 1.34
> +++ usr.sbin/ntpd//constraint.c 2 Dec 2016 16:27:15 -0000
> @@ -321,8 +321,8 @@ priv_constraint_readquery(struct constra
> void
> priv_constraint_child(const char *pw_dir, uid_t pw_uid, gid_t pw_gid)
> {
> - struct constraint *cstr;
> - struct ntp_addr_msg *am;
> + struct constraint cstr;
> + struct ntp_addr_msg am;
> uint8_t *data;
> static char addr[NI_MAXHOST];
> struct timeval rectv, xmttv;
> @@ -336,10 +336,6 @@ priv_constraint_child(const char *pw_dir
> if (setpriority(PRIO_PROCESS, 0, 0) == -1)
> log_warn("could not set priority");
>
> - if ((cstr = calloc(1, sizeof(*cstr))) == NULL ||
> - (am = calloc(1, sizeof(*am))) == NULL)
> - fatal("%s: calloc", __func__);
> -
> /* Init TLS and load CA certs before chroot() */
> if (tls_init() == -1)
> fatalx("tls_init");
> @@ -368,9 +364,9 @@ priv_constraint_child(const char *pw_dir
> if (pledge("stdio inet", NULL) == -1)
> fatal("pledge");
>
> - cstr->fd = CONSTRAINT_PASSFD;
> - imsg_init(&cstr->ibuf, cstr->fd);
> - priv_constraint_readquery(cstr, am, &data);
> + cstr.fd = CONSTRAINT_PASSFD;
> + imsg_init(&cstr.ibuf, cstr.fd);
> + priv_constraint_readquery(&cstr, &am, &data);
>
> /*
> * Get the IP address as name and set the process title
> accordingly.
> @@ -378,8 +374,8 @@ priv_constraint_child(const char *pw_dir
> * any DNS operation, so it is safe to be called without the dns
> * pledge.
> */
> - if (getnameinfo((struct sockaddr *)&cstr->addr->ss,
> - SA_LEN((struct sockaddr *)&cstr->addr->ss),
> + if (getnameinfo((struct sockaddr *)&cstr.addr->ss,
> + SA_LEN((struct sockaddr *)&cstr.addr->ss),
> addr, sizeof(addr), NULL, 0,
> NI_NUMERICHOST) != 0)
> fatalx("%s getnameinfo", __func__);
> @@ -398,21 +394,21 @@ priv_constraint_child(const char *pw_dir
> fatal("%s fcntl F_SETFD", __func__);
>
> /* Get remaining data from imsg in the unpriv child */
> - if (am->namelen) {
> - if ((cstr->addr_head.name =
> - get_string(data, am->namelen)) == NULL)
> + if (am.namelen) {
> + if ((cstr.addr_head.name =
> + get_string(data, am.namelen)) == NULL)
> fatalx("invalid IMSG_CONSTRAINT_QUERY name");
> - data += am->namelen;
> + data += am.namelen;
> }
> - if (am->pathlen) {
> - if ((cstr->addr_head.path =
> - get_string(data, am->pathlen)) == NULL)
> + if (am.pathlen) {
> + if ((cstr.addr_head.path =
> + get_string(data, am.pathlen)) == NULL)
> fatalx("invalid IMSG_CONSTRAINT_QUERY path");
> }
>
> /* Run! */
> if ((ctx = httpsdate_query(addr,
> - CONSTRAINT_PORT, cstr->addr_head.name, cstr->addr_head.path,
> + CONSTRAINT_PORT, cstr.addr_head.name, cstr.addr_head.path,
> conf->ca, conf->ca_len, &rectv, &xmttv)) == NULL) {
> /* Abort with failure but without warning */
> exit(1);
> @@ -422,10 +418,10 @@ priv_constraint_child(const char *pw_dir
> iov[0].iov_len = sizeof(rectv);
> iov[1].iov_base = &xmttv;
> iov[1].iov_len = sizeof(xmttv);
> - imsg_composev(&cstr->ibuf,
> + imsg_composev(&cstr.ibuf,
> IMSG_CONSTRAINT_RESULT, 0, 0, -1, iov, 2);
> do {
> - rv = imsg_flush(&cstr->ibuf);
> + rv = imsg_flush(&cstr.ibuf);
> } while (rv == -1 && errno == EAGAIN);
>
> /* Tear down the TLS connection after sending the result */
>
>
