system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-09-30 13:44:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

ctld: fix several process setup/teardown bugs

Browse source 
All of the below bugs could result in a system where ctld is not
running, but LUNs and targets still exist in the kernel; a difficult
situation to recover from.

* open the pidfile earlier.  Open the pidfile before reading the
  kernel's current state, so two racing ctld processes won't step on
  each others' toes.

* close the pidfile later.  Close it after tearing down the
  configuration, for the same reason.

* If the configured pidfile changes, then rename it on SIGHUP rather
  than remove and recreate it.

* When running in debug mode, don't close the pidfile while handling a
  new connection.  Only do that in non-debug mode, in the child of the
  fork.

* Register signal handlers earlier.  Otherwise a SIGTERM signal received
  during startup could kill ctld without tearing down the configuration.

MFC after:	2 weeks
PR:		271460
Sponsored by:	Axcient
Reviewed by:	mav
Pull Request:	https://github.com/freebsd/freebsd-src/pull/1370
This commit is contained in:
Alan Somers 2024-08-07 09:21:08 -06:00
parent ce9c3abf69
commit 5f89aea7b7
1 changed files with 40 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
usr.sbin/ctld/ctld.c
View file

@ -1915,7 +1915,6 @@ conf_apply(struct conf *oldconf, struct conf *newconf)
struct portal *oldp, *newp; struct portal *oldp, *newp;
struct port *oldport, *newport, *tmpport; struct port *oldport, *newport, *tmpport;
struct isns *oldns, *newns; struct isns *oldns, *newns;
pid_t otherpid;
int changed, cumulated_error = 0, error, sockbuf; int changed, cumulated_error = 0, error, sockbuf;
int one = 1; int one = 1;
@ -1924,32 +1923,25 @@ conf_apply(struct conf *oldconf, struct conf *newconf)
log_init(newconf->conf_debug); log_init(newconf->conf_debug);
} }
if (oldconf->conf_pidfh != NULL) { if (oldconf->conf_pidfile_path != NULL &&
assert(oldconf->conf_pidfile_path != NULL); newconf->conf_pidfile_path != NULL)
if (newconf->conf_pidfile_path != NULL && {
strcmp(oldconf->conf_pidfile_path, if (strcmp(oldconf->conf_pidfile_path,
newconf->conf_pidfile_path) == 0) { newconf->conf_pidfile_path) != 0)
newconf->conf_pidfh = oldconf->conf_pidfh; {
oldconf->conf_pidfh = NULL; /* pidfile has changed. rename it */
} else { log_debugx("moving pidfile to %s",
log_debugx("removing pidfile %s", newconf->conf_pidfile_path);
oldconf->conf_pidfile_path); if (rename(oldconf->conf_pidfile_path,
pidfile_remove(oldconf->conf_pidfh); newconf->conf_pidfile_path))
oldconf->conf_pidfh = NULL; {
} log_err(1, "renaming pidfile %s -> %s",
} oldconf->conf_pidfile_path,
newconf->conf_pidfile_path);
if (newconf->conf_pidfh == NULL && newconf->conf_pidfile_path != NULL) { }
log_debugx("opening pidfile %s", newconf->conf_pidfile_path);
newconf->conf_pidfh =
pidfile_open(newconf->conf_pidfile_path, 0600, &otherpid);
if (newconf->conf_pidfh == NULL) {
if (errno == EEXIST)
log_errx(1, "daemon already running, pid: %jd.",
(intmax_t)otherpid);
log_err(1, "cannot open or create pidfile \"%s\"",
newconf->conf_pidfile_path);
} }
newconf->conf_pidfh = oldconf->conf_pidfh;
oldconf->conf_pidfh = NULL;
} }
/* /*
@ -2471,8 +2463,8 @@ handle_connection(struct portal *portal, int fd,
close(fd); close(fd);
return; return;
} }
pidfile_close(conf->conf_pidfh);
} }
pidfile_close(conf->conf_pidfh);
error = getnameinfo(client_sa, client_sa->sa_len, error = getnameinfo(client_sa, client_sa->sa_len,
host, sizeof(host), NULL, 0, NI_NUMERICHOST); host, sizeof(host), NULL, 0, NI_NUMERICHOST);
@ -2807,6 +2799,7 @@ main(int argc, char **argv)
struct isns *newns; struct isns *newns;
const char *config_path = DEFAULT_CONFIG_PATH; const char *config_path = DEFAULT_CONFIG_PATH;
int debug = 0, ch, error; int debug = 0, ch, error;
pid_t otherpid;
bool dont_daemonize = false; bool dont_daemonize = false;
bool test_config = false; bool test_config = false;
bool use_ucl = false; bool use_ucl = false;
@ -2846,7 +2839,6 @@ main(int argc, char **argv)
kernel_init(); kernel_init();
TAILQ_INIT(&kports.pports); TAILQ_INIT(&kports.pports);
oldconf = conf_new_from_kernel(&kports);
newconf = conf_new_from_file(config_path, use_ucl); newconf = conf_new_from_file(config_path, use_ucl);
if (newconf == NULL) if (newconf == NULL)
@ -2855,6 +2847,22 @@ main(int argc, char **argv)
if (test_config) if (test_config)
return (0); return (0);
assert(newconf->conf_pidfile_path != NULL);
log_debugx("opening pidfile %s", newconf->conf_pidfile_path);
newconf->conf_pidfh = pidfile_open(newconf->conf_pidfile_path, 0600,
&otherpid);
if (newconf->conf_pidfh == NULL) {
if (errno == EEXIST)
log_errx(1, "daemon already running, pid: %jd.",
(intmax_t)otherpid);
log_err(1, "cannot open or create pidfile \"%s\"",
newconf->conf_pidfile_path);
}
register_signals();
oldconf = conf_new_from_kernel(&kports);
if (debug > 0) { if (debug > 0) {
oldconf->conf_debug = debug; oldconf->conf_debug = debug;
newconf->conf_debug = debug; newconf->conf_debug = debug;
@ -2870,8 +2878,6 @@ main(int argc, char **argv)
conf_delete(oldconf); conf_delete(oldconf);
oldconf = NULL; oldconf = NULL;
register_signals();
if (dont_daemonize == false) { if (dont_daemonize == false) {
log_debugx("daemonizing"); log_debugx("daemonizing");
if (daemon(0, 0) == -1) { if (daemon(0, 0) == -1) {
@ -2926,6 +2932,10 @@ main(int argc, char **argv)
error = conf_apply(oldconf, newconf); error = conf_apply(oldconf, newconf);
if (error != 0) if (error != 0)
log_warnx("failed to apply configuration"); log_warnx("failed to apply configuration");
if (oldconf->conf_pidfh) {
pidfile_remove(oldconf->conf_pidfh);
oldconf->conf_pidfh = NULL;
}
conf_delete(newconf); conf_delete(newconf);
conf_delete(oldconf); conf_delete(oldconf);
oldconf = NULL; oldconf = NULL;