From: Kirill A. Korinsky Subject: Re: httpd: remove control.c To: Martijn van Duren Cc: tech@openbsd.org Date: Wed, 01 Jul 2026 19:47:26 +0200 On Wed, 01 Jul 2026 14:34:20 +0200, Martijn van Duren wrote: > > Hello tech@, > > As far as I'm aware there's no httpctl, nor have I heard of anyone with > the intention of writing it. So instead of just removing ps_rcsocks like > was done in vmd, and I just proposed for relayd: simply remove all the > control code as was done for snmpd years ago. > > OK? OK kirill@ > > martijn@ > > diff d010102fe4efde3ee95164581695909c7c6b672a dd90a38340399a0dae680b481ce25f7229346999 > commit - d010102fe4efde3ee95164581695909c7c6b672a > commit + dd90a38340399a0dae680b481ce25f7229346999 > blob - a99616ec18a2c0ae42bdf0428ea83ca1b933624c > blob + b723a4d21c7be66c514461e0b8d1cc16fa9f923d > --- usr.sbin/httpd/Makefile > +++ usr.sbin/httpd/Makefile > @@ -3,7 +3,7 @@ > PROG= httpd > RELINK= "./${PROG} -n -f /etc/examples/httpd.conf 2> /dev/null" > SRCS= parse.y > -SRCS+= config.c control.c httpd.c log.c logger.c proc.c > +SRCS+= config.c httpd.c log.c logger.c proc.c > SRCS+= server.c server_http.c server_file.c server_fcgi.c > MAN= httpd.8 httpd.conf.5 > > blob - 6bbc9a4ce83c6668287d5ab45aaca61d74a51484 (mode 644) > blob + /dev/null > --- usr.sbin/httpd/control.c > +++ /dev/null > @@ -1,319 +0,0 @@ > -/* $OpenBSD: control.c,v 1.22 2026/03/02 19:24:58 rsadowski Exp $ */ > - > -/* > - * Copyright (c) 2003, 2004 Henning Brauer > - * > - * Permission to use, copy, modify, and distribute this software for any > - * purpose with or without fee is hereby granted, provided that the above > - * copyright notice and this permission notice appear in all copies. > - * > - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > - */ > - > -#include > -#include > -#include > -#include > -#include > - > -#include > -#include > -#include > -#include > -#include > -#include > -#include > - > -#include "httpd.h" > -#include "log.h" > - > -#define CONTROL_BACKLOG 5 > - > -struct ctl_connlist ctl_conns = TAILQ_HEAD_INITIALIZER(ctl_conns); > - > -void control_accept(int, short, void *); > -void control_close(int, struct control_sock *); > - > -int > -control_init(struct privsep *ps, struct control_sock *cs) > -{ > - struct httpd *env = ps->ps_env; > - struct sockaddr_un sun; > - int fd; > - mode_t old_umask, mode; > - > - if (cs->cs_name == NULL) > - return (0); > - > - if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0)) == -1) { > - log_warn("%s: socket", __func__); > - return (-1); > - } > - > - sun.sun_family = AF_UNIX; > - if (strlcpy(sun.sun_path, cs->cs_name, > - sizeof(sun.sun_path)) >= sizeof(sun.sun_path)) { > - log_warn("%s: %s name too long", __func__, cs->cs_name); > - close(fd); > - return (-1); > - } > - > - if (unlink(cs->cs_name) == -1) > - if (errno != ENOENT) { > - log_warn("%s: unlink %s", __func__, cs->cs_name); > - close(fd); > - return (-1); > - } > - > - if (cs->cs_restricted) { > - old_umask = umask(S_IXUSR|S_IXGRP|S_IXOTH); > - mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH; > - } else { > - old_umask = umask(S_IXUSR|S_IXGRP|S_IWOTH|S_IROTH|S_IXOTH); > - mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP; > - } > - > - if (bind(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1) { > - log_warn("%s: bind: %s", __func__, cs->cs_name); > - close(fd); > - (void)umask(old_umask); > - return (-1); > - } > - (void)umask(old_umask); > - > - if (chmod(cs->cs_name, mode) == -1) { > - log_warn("%s: chmod", __func__); > - close(fd); > - (void)unlink(cs->cs_name); > - return (-1); > - } > - > - cs->cs_fd = fd; > - cs->cs_env = env; > - > - return (0); > -} > - > -int > -control_listen(struct control_sock *cs) > -{ > - if (cs->cs_name == NULL) > - return (0); > - > - if (listen(cs->cs_fd, CONTROL_BACKLOG) == -1) { > - log_warn("%s: listen", __func__); > - return (-1); > - } > - > - event_set(&cs->cs_ev, cs->cs_fd, EV_READ, > - control_accept, cs); > - event_add(&cs->cs_ev, NULL); > - evtimer_set(&cs->cs_evt, control_accept, cs); > - > - return (0); > -} > - > -void > -control_cleanup(struct control_sock *cs) > -{ > - if (cs->cs_name == NULL) > - return; > - event_del(&cs->cs_ev); > - event_del(&cs->cs_evt); > -} > - > -void > -control_accept(int listenfd, short event, void *arg) > -{ > - int connfd; > - socklen_t len; > - struct sockaddr_un sun; > - struct ctl_conn *c; > - struct control_sock *cs = arg; > - > - event_add(&cs->cs_ev, NULL); > - if ((event & EV_TIMEOUT)) > - return; > - > - len = sizeof(sun); > - if ((connfd = accept4(listenfd, > - (struct sockaddr *)&sun, &len, SOCK_NONBLOCK)) == -1) { > - /* > - * Pause accept if we are out of file descriptors, or > - * libevent will haunt us here too. > - */ > - if (errno == ENFILE || errno == EMFILE) { > - struct timeval evtpause = { 1, 0 }; > - > - event_del(&cs->cs_ev); > - evtimer_add(&cs->cs_evt, &evtpause); > - } else if (errno != EWOULDBLOCK && errno != EINTR && > - errno != ECONNABORTED) > - log_warn("%s: accept", __func__); > - return; > - } > - > - if ((c = calloc(1, sizeof(struct ctl_conn))) == NULL) { > - log_warn("%s: calloc", __func__); > - close(connfd); > - return; > - } > - > - if (imsgbuf_init(&c->iev.ibuf, connfd) == -1) { > - log_warn("%s: imsgbuf_init", __func__); > - close(connfd); > - free(c); > - return; > - } > - c->iev.handler = control_dispatch_imsg; > - c->iev.events = EV_READ; > - c->iev.data = cs; /* proc.c cheats (reuses the handler) */ > - event_set(&c->iev.ev, c->iev.ibuf.fd, c->iev.events, > - c->iev.handler, cs); > - event_add(&c->iev.ev, NULL); > - > - TAILQ_INSERT_TAIL(&ctl_conns, c, entry); > -} > - > -struct ctl_conn * > -control_connbyfd(int fd) > -{ > - struct ctl_conn *c; > - > - TAILQ_FOREACH(c, &ctl_conns, entry) { > - if (c->iev.ibuf.fd == fd) > - break; > - } > - > - return (c); > -} > - > -void > -control_close(int fd, struct control_sock *cs) > -{ > - struct ctl_conn *c; > - > - if ((c = control_connbyfd(fd)) == NULL) { > - log_warn("%s: fd %d not found", __func__, fd); > - return; > - } > - > - imsgbuf_clear(&c->iev.ibuf); > - TAILQ_REMOVE(&ctl_conns, c, entry); > - > - event_del(&c->iev.ev); > - close(c->iev.ibuf.fd); > - > - /* Some file descriptors are available again. */ > - if (evtimer_pending(&cs->cs_evt, NULL)) { > - evtimer_del(&cs->cs_evt); > - event_add(&cs->cs_ev, NULL); > - } > - > - free(c); > -} > - > -void > -control_dispatch_imsg(int fd, short event, void *arg) > -{ > - struct control_sock *cs = arg; > - struct ctl_conn *c; > - struct imsg imsg; > - int n; > - int verbose; > - struct httpd *env = cs->cs_env; > - > - if ((c = control_connbyfd(fd)) == NULL) { > - log_warn("%s: fd %d not found", __func__, fd); > - return; > - } > - > - if (event & EV_READ) { > - if (imsgbuf_read(&c->iev.ibuf) != 1) { > - control_close(fd, cs); > - return; > - } > - } > - > - if (event & EV_WRITE) { > - if (imsgbuf_write(&c->iev.ibuf) == -1) { > - control_close(fd, cs); > - return; > - } > - } > - > - for (;;) { > - if ((n = imsg_get(&c->iev.ibuf, &imsg)) == -1) { > - control_close(fd, cs); > - return; > - } > - > - if (n == 0) > - break; > - > - if (c->waiting) { > - log_debug("%s: unexpected imsg %d", > - __func__, imsg.hdr.type); > - imsg_free(&imsg); > - control_close(fd, cs); > - return; > - } > - > - switch (imsg.hdr.type) { > - case IMSG_CTL_SHUTDOWN: > - case IMSG_CTL_RELOAD: > - case IMSG_CTL_REOPEN: > - proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT, -1); > - break; > - case IMSG_CTL_NOTIFY: > - if (c->flags & CTL_CONN_NOTIFY) { > - log_debug("%s: " > - "client requested notify more than once", > - __func__); > - imsg_compose_event(&c->iev, IMSG_CTL_FAIL, > - 0, env->sc_ps->ps_instance + 1, -1, > - NULL, 0); > - break; > - } > - c->flags |= CTL_CONN_NOTIFY; > - break; > - case IMSG_CTL_VERBOSE: > - IMSG_SIZE_CHECK(&imsg, &verbose); > - > - memcpy(&verbose, imsg.data, sizeof(verbose)); > - > - proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT, -1); > - proc_forward_imsg(env->sc_ps, &imsg, PROC_SERVER, -1); > - > - memcpy(imsg.data, &verbose, sizeof(verbose)); > - control_imsg_forward(env->sc_ps, &imsg); > - log_setverbose(verbose); > - break; > - default: > - log_debug("%s: error handling imsg %d", > - __func__, imsg.hdr.type); > - break; > - } > - imsg_free(&imsg); > - } > - > - imsg_event_add(&c->iev); > -} > - > -void > -control_imsg_forward(struct privsep *ps, struct imsg *imsg) > -{ > - struct ctl_conn *c; > - > - TAILQ_FOREACH(c, &ctl_conns, entry) > - if (c->flags & CTL_CONN_NOTIFY) > - imsg_compose_event(&c->iev, imsg->hdr.type, > - 0, ps->ps_instance + 1, -1, imsg->data, > - imsg->hdr.len - IMSG_HEADER_SIZE); > -} > blob - d0188f6acb81f9d5fd38b7877e8b0c406b26eaba > blob + fb0db77542f92822a3b593439034ae0ceb09d077 > --- usr.sbin/httpd/httpd.c > +++ usr.sbin/httpd/httpd.c > @@ -179,7 +179,6 @@ main(int argc, char *argv[]) > httpd_env = env; > env->sc_ps = ps; > ps->ps_env = env; > - TAILQ_INIT(&ps->ps_rcsocks); > env->sc_conffile = conffile; > env->sc_opts = opts; > > @@ -192,9 +191,6 @@ main(int argc, char *argv[]) > if ((ps->ps_pw = getpwnam(HTTPD_USER)) == NULL) > errx(1, "unknown user %s", HTTPD_USER); > > - /* Configure the control socket */ > - ps->ps_csock.cs_name = NULL; > - > log_init(debug, LOG_DAEMON); > log_setverbose(verbose); > > @@ -395,10 +391,6 @@ parent_shutdown(struct httpd *env) > config_purge(env, CONFIG_ALL); > > proc_kill(env->sc_ps); > - control_cleanup(&env->sc_ps->ps_csock); > - if (env->sc_ps->ps_csock.cs_name != NULL) > - (void)unlink(env->sc_ps->ps_csock.cs_name); > - > free(env->sc_ps); > free(env); > > blob - e4cac855b4b5b1bbfa71efddd03660dee34c81b5 > blob + 4cd8c09037cbd93b205d28f56f77991af3aea124 > --- usr.sbin/httpd/httpd.h > +++ usr.sbin/httpd/httpd.h > @@ -153,19 +153,6 @@ struct address { > }; > TAILQ_HEAD(addresslist, address); > > -/* initially control.h */ > -struct control_sock { > - const char *cs_name; > - struct event cs_ev; > - struct event cs_evt; > - int cs_fd; > - int cs_restricted; > - void *cs_env; > - > - TAILQ_ENTRY(control_sock) cs_entry; > -}; > -TAILQ_HEAD(control_socks, control_sock); > - > struct imsgev { > struct imsgbuf ibuf; > void (*handler)(int, short, void *); > @@ -182,16 +169,6 @@ struct imsgev { > #define IMSG_DATA_SIZE(imsg) ((imsg)->hdr.len - IMSG_HEADER_SIZE) > #define MAX_IMSG_DATA_SIZE (MAX_IMSGSIZE - IMSG_HEADER_SIZE) > > -struct ctl_conn { > - TAILQ_ENTRY(ctl_conn) entry; > - uint8_t flags; > - unsigned int waiting; > -#define CTL_CONN_NOTIFY 0x01 > - struct imsgev iev; > - > -}; > -TAILQ_HEAD(ctl_connlist, ctl_conn); > - > enum imsg_type { > IMSG_NONE, > IMSG_CTL_OK, > @@ -226,9 +203,6 @@ enum privsep_procid { > }; > extern enum privsep_procid privsep_process; > > -/* Attach the control socket to the following process */ > -#define PROC_CONTROL PROC_LOGGER > - > struct privsep_pipes { > int *pp_pipes[PROC_MAX]; > }; > @@ -244,9 +218,6 @@ struct privsep { > unsigned int ps_instances[PROC_MAX]; > unsigned int ps_instance; > > - struct control_sock ps_csock; > - struct control_socks ps_rcsocks; > - > /* Event and signal handlers */ > struct event ps_evsigint; > struct event ps_evsigterm; > @@ -613,15 +584,6 @@ struct httpd { > #define HTTPD_OPT_VERBOSE 0x01 > #define HTTPD_OPT_NOACTION 0x04 > > -/* control.c */ > -int control_init(struct privsep *, struct control_sock *); > -int control_listen(struct control_sock *); > -void control_cleanup(struct control_sock *); > -void control_dispatch_imsg(int, short, void *); > -void control_imsg_forward(struct privsep *, struct imsg *); > -struct ctl_conn * > - control_connbyfd(int); > - > /* parse.y */ > int parse_config(const char *, struct httpd *); > int load_config(const char *, struct httpd *); > blob - e75950c1f49373c6480a44f9de376a8111b0959f > blob + eae6aa7e53852e60ea1918c81bbfe866afa4980e > --- usr.sbin/httpd/proc.c > +++ usr.sbin/httpd/proc.c > @@ -474,9 +474,6 @@ proc_shutdown(struct privsep_proc *p) > { > struct privsep *ps = p->p_ps; > > - if (p->p_id == PROC_CONTROL && ps) > - control_cleanup(&ps->ps_csock); > - > if (p->p_shutdown != NULL) > (*p->p_shutdown)(); > > @@ -516,18 +513,9 @@ proc_run(struct privsep *ps, struct privsep_proc *p, > { > struct passwd *pw; > const char *root; > - struct control_sock *rcs; > > log_procinit(p->p_title); > > - if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) { > - if (control_init(ps, &ps->ps_csock) == -1) > - fatalx("%s: control_init", __func__); > - TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry) > - if (control_init(ps, rcs) == -1) > - fatalx("%s: control_init", __func__); > - } > - > /* Use non-standard user */ > if (p->p_pw != NULL) > pw = p->p_pw; > @@ -572,13 +560,6 @@ proc_run(struct privsep *ps, struct privsep_proc *p, > > proc_setup(ps, procs, nproc); > proc_accept(ps, PROC_PARENT_SOCK_FILENO, PROC_PARENT, 0); > - if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) { > - if (control_listen(&ps->ps_csock) == -1) > - fatalx("%s: control_listen", __func__); > - TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry) > - if (control_listen(rcs) == -1) > - fatalx("%s: control_listen", __func__); > - } > > DPRINTF("%s: %s %d/%d, pid %d", __func__, p->p_title, > ps->ps_instance + 1, ps->ps_instances[p->p_id], getpid()); > -- wbr, Kirill