Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: httpd: remove control.c
To:
Martijn van Duren <openbsd+tech@list.imperialat.at>
Cc:
tech@openbsd.org
Date:
Wed, 01 Jul 2026 19:47:26 +0200

Download raw body.

Thread
  • Martijn van Duren:

    httpd: remove control.c

    • Kirill A. Korinsky:

      httpd: remove control.c

On Wed, 01 Jul 2026 14:34:20 +0200,
Martijn van Duren <openbsd+tech@list.imperialat.at> 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 <henning@openbsd.org>
> - *
> - * 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 <sys/queue.h>
> -#include <sys/stat.h>
> -#include <sys/socket.h>
> -#include <sys/time.h>
> -#include <sys/un.h>
> -
> -#include <errno.h>
> -#include <event.h>
> -#include <fcntl.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <unistd.h>
> -#include <imsg.h>
> -
> -#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