Hi,
It seems that switchd(8) suffers from the same issue I reported yesterday about
eigrpd(8), the daemon when it's shutdown never deletes the unix control socket
because that is being done from a chrooted process.
This one nevertheless took a little bit more effort since I had to disentangle
2 struct event and 1 fd from struct control_sock into their own
struct control_state, just like we have on other daemons.
Also updated the pledge(2)s accordingly moving "cpath" from the chrooted proc
to the main proc.
Regression tests passed without issues and switchctl(8) also didn't complain.
Comments? OK?
Index: control.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/control.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 control.c
--- control.c 17 Jan 2017 22:10:56 -0000 1.8
+++ control.c 2 Aug 2018 09:04:28 -0000
@@ -70,11 +70,10 @@ control_run(struct privsep *ps, struct p
/*
* pledge in the control process:
* stdio - for malloc and basic I/O including events.
- * cpath - for managing the control socket.
* unix - for the control socket.
* recvfd - for the proc fd exchange.
*/
- if (pledge("stdio cpath unix recvfd", NULL) == -1)
+ if (pledge("stdio unix recvfd", NULL) == -1)
fatal("pledge");
}
@@ -171,7 +170,7 @@ control_init(struct privsep *ps, struct
}
socket_set_blockmode(fd, BM_NONBLOCK);
- cs->cs_fd = fd;
+ control_state.fd = fd;
cs->cs_env = env;
return (0);
@@ -183,27 +182,27 @@ control_listen(struct control_sock *cs)
if (cs->cs_name == NULL)
return (0);
- if (listen(cs->cs_fd, CONTROL_BACKLOG) == -1) {
+ if (listen(control_state.fd, CONTROL_BACKLOG) == -1) {
log_warn("%s: listen", __func__);
return (-1);
}
- event_set(&cs->cs_ev, cs->cs_fd, EV_READ,
+ event_set(&control_state.ev, control_state.fd, EV_READ,
control_accept, cs);
- event_add(&cs->cs_ev, NULL);
- evtimer_set(&cs->cs_evt, control_accept, cs);
+ event_add(&control_state.ev, NULL);
+ evtimer_set(&control_state.evt, control_accept, cs);
return (0);
}
void
-control_cleanup(struct control_sock *cs)
+control_cleanup(char *path)
{
- if (cs->cs_name == NULL)
+ if (path == NULL)
return;
- event_del(&cs->cs_ev);
- event_del(&cs->cs_evt);
- (void)unlink(cs->cs_name);
+ event_del(&control_state.ev);
+ event_del(&control_state.evt);
+ (void)unlink(path);
}
/* ARGSUSED */
@@ -216,7 +215,7 @@ control_accept(int listenfd, short event
struct sockaddr_un sun;
struct ctl_conn *c;
- event_add(&cs->cs_ev, NULL);
+ event_add(&control_state.ev, NULL);
if ((event & EV_TIMEOUT))
return;
@@ -230,8 +229,8 @@ control_accept(int listenfd, short event
if (errno == ENFILE || errno == EMFILE) {
struct timeval evtpause = { 1, 0 };
- event_del(&cs->cs_ev);
- evtimer_add(&cs->cs_evt, &evtpause);
+ event_del(&control_state.ev);
+ evtimer_add(&control_state.evt, &evtpause);
} else if (errno != EWOULDBLOCK && errno != EINTR &&
errno != ECONNABORTED)
log_warn("%s: accept", __func__);
@@ -287,9 +286,9 @@ control_close(int fd, struct control_soc
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);
+ if (evtimer_pending(&control_state.evt, NULL)) {
+ evtimer_del(&control_state.evt);
+ event_add(&control_state.ev, NULL);
}
free(c);
Index: proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 proc.c
--- proc.c 29 May 2017 12:56:26 -0000 1.12
+++ proc.c 2 Aug 2018 09:04:28 -0000
@@ -475,9 +475,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)();
Index: proc.h
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.h,v
retrieving revision 1.6
diff -u -p -u -r1.6 proc.h
--- proc.h 9 Jan 2017 14:49:22 -0000 1.6
+++ proc.h 2 Aug 2018 09:04:28 -0000
@@ -23,11 +23,14 @@
#ifndef _PROC_H
#define _PROC_H
+struct {
+ struct event ev;
+ struct event evt;
+ int fd;
+} control_state;
+
struct control_sock {
const char *cs_name;
- struct event cs_ev;
- struct event cs_evt;
- int cs_fd;
int cs_restricted;
void *cs_env;
@@ -160,7 +163,7 @@ int proc_flush_imsg(struct privsep *, e
/* control.c */
int control_init(struct privsep *, struct control_sock *);
int control_listen(struct control_sock *);
-void control_cleanup(struct control_sock *);
+void control_cleanup(char *);
struct ctl_conn
*control_connbyfd(int);
void control(struct privsep *, struct privsep_proc *);
Index: switchd.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/switchd.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 switchd.c
--- switchd.c 9 Jan 2017 14:49:22 -0000 1.15
+++ switchd.c 2 Aug 2018 09:04:28 -0000
@@ -196,11 +196,12 @@ main(int argc, char *argv[])
* stdio - for malloc and basic I/O including events.
* rpath - for reload to open and read the configuration files.
* wpath - for accessing the /dev/switch device.
+ * cpath - for managing the control socket.
* inet - for opening OpenFlow and device sockets.
* dns - for resolving host in the configuration files.
* sendfd - send sockets to child processes on reload.
*/
- if (pledge("stdio rpath wpath inet dns sendfd", NULL) == -1)
+ if (pledge("stdio rpath wpath cpath inet dns sendfd", NULL) == -1)
fatal("pledge");
event_init();
@@ -472,6 +473,8 @@ parent_dispatch_control(int fd, struct p
void
parent_shutdown(struct switchd *sc)
{
+ control_cleanup(SWITCHD_SOCKET);
+
proc_kill(&sc->sc_ps);
free(sc);
