Extend sendsignal*() API some more to optionally fall back to checkpr…

…ocname() vs current getpid() [networkupstools#2463]

Signed-off-by: Jim Klimov <[email protected]>

NEWS.adoc

@@ -145,7 +145,8 @@ https://github.com/networkupstools/nut/milestone/11
      significantly from a `progname` string defined in the respective NUT
      source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment
      variable support was added to optionally disable this verification.
-     [issue #2463]
+     Also the NUT daemons should request to double-check against their
+     run-time process name (if it can be detected). [issue #2463]
 
  - various recipe, documentation and source files were revised to address
    respective warnings issued by the new generations of analysis tools.

UPGRADING.adoc

@@ -46,7 +46,8 @@ Changes from 2.8.2 to 2.8.3
   names differ significantly from a `progname` defined in the respective
   NUT source file, so a boolean `NUT_IGNORE_CHECKPROCNAME` environment
   variable support was added to optionally disable this verification.
-  [issue #2463]
+  Also the NUT daemons should request to double-check against their
+  run-time process name (if it can be detected). [issue #2463]
 
 
 Changes from 2.8.1 to 2.8.2

clients/upsmon.c

@@ -3022,15 +3022,15 @@ int main(int argc, char *argv[])
 	 * is running by sending signal '0' (i.e. 'kill <pid> 0' equivalent)
 	 */
 	if (oldpid < 0) {
-		cmdret = sendsignal(prog, cmd);
+		cmdret = sendsignal(prog, cmd, 1);
 	} else {
-		cmdret = sendsignalpid(oldpid, cmd, prog);
+		cmdret = sendsignalpid(oldpid, cmd, prog, 1);
 	}
 
 #else	/* WIN32 */
 	if (cmd) {
 		/* Command the running daemon, it should be there */
-		cmdret = sendsignal(UPSMON_PIPE_NAME, cmd);
+		cmdret = sendsignal(UPSMON_PIPE_NAME, cmd, 1);
 	} else {
 		/* Starting new daemon, check for competition */
 		mutex = CreateMutex(NULL, TRUE, UPSMON_PIPE_NAME);

common/common.c

@@ -1150,10 +1150,11 @@ void writepid(const char *name)
 /* send sig to pid, returns -1 for error, or
  * zero for a successfully sent signal
  */
-int sendsignalpid(pid_t pid, int sig, const char *progname)
+int sendsignalpid(pid_t pid, int sig, const char *progname, int check_current_progname)
 {
 #ifndef WIN32
-	int	ret;
+	int	ret, cpn1 = -10, cpn2 = -10;
+	char	*current_progname = NULL;
 
 	/* TOTHINK: What about containers where a NUT daemon *is* the only process
 	 * and is the PID=1 of the container (recycle if dead)? */
@@ -1165,13 +1166,114 @@ int sendsignalpid(pid_t pid, int sig, const char *progname)
 		return -1;
 	}
 
-	if (progname && !checkprocname(pid, progname)) {
-		if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1)
-			upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
-				" which exists but is not of program '%s'",
-				(uintmax_t)pid, progname);
+	ret = 0;
+	if (progname) {
+		/* Check against some expected (often built-in) name */
+		if (!(cpn1 = checkprocname(pid, progname))) {
+			/* Did not match expected (often built-in) name */
+			ret = -1;
+		} else {
+			if (cpn1 > 0) {
+				/* Matched expected name, ok to proceed */
+				ret = 1;
+			}
+			/* ...else could not determine name of PID; think later */
+		}
+	}
+	/* if (cpn1 == -3) => NUT_IGNORE_CHECKPROCNAME=true */
+	/* if (cpn1 == -1) => could not determine name of PID... retry just in case? */
+	if (ret <= 0 && check_current_progname && cpn1 != -3) {
+		/* NOTE: This could be optimized a bit by pre-finding the procname
+		 * of "pid" and re-using it, but this is not a hot enough code path
+		 * to bother much.
+		 */
+		current_progname = getprocname(getpid());
+		if (current_progname && (cpn2 = checkprocname(pid, current_progname))) {
+			if (cpn2 > 0) {
+				/* Matched current process as asked, ok to proceed */
+				ret = 2;
+			}
+			/* ...else could not determine name of PID; think later */
+		} else {
+			if (current_progname) {
+				/* Did not match current process name */
+				ret = -2;
+			} /* else just did not determine current process
+			   * name, so did not actually check either
+			   * // ret = -3;
+			   */
+		}
+	}
+
+	/* if ret == 0, ok to proceed - not asked for any sanity checks;
+	 * if ret > 0 we had some definitive match above
+	 */
+	if (ret < 0) {
+		upsdebugx(1,
+			"%s: ran at least one check, and all such checks "
+			"found a process name for PID %" PRIuMAX " and "
+			"failed to match: expected progname='%s' (res=%d), "
+			"current progname='%s' (res=%d)",
+			__func__, (uintmax_t)pid,
+			NUT_STRARG(progname), cpn1,
+			NUT_STRARG(current_progname), cpn2);
+
+		if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1) {
+			switch (ret) {
+				case -1:
+					upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
+						" which exists but is not of"
+						" expected program '%s'; not asked"
+						" to cross-check current PID's name",
+						(uintmax_t)pid, progname);
+					break;
+
+				/* Maybe we tried both data sources, maybe just current_progname */
+				case -2:
+				/*case -3:*/
+					if (progname && current_progname) {
+						/* Tried both, downgraded verdict further */
+						upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
+							" which exists but is not of expected"
+							" program '%s' nor current '%s'",
+							(uintmax_t)pid, progname, current_progname);
+					} else if (current_progname) {
+						/* Not asked for progname==NULL */
+						upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
+							" which exists but is not of"
+							" current program '%s'",
+							(uintmax_t)pid, current_progname);
+					} else if (progname) {
+						upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
+							" which exists but is not of"
+							" expected program '%s'; could not"
+							" cross-check current PID's name",
+							(uintmax_t)pid, progname);
+					} else {
+						/* Both NULL; one not asked, another not detected;
+						 * should not actually get here (wannabe `ret==-3`)
+						 */
+						upslogx(LOG_ERR, "Tried to signal PID %" PRIuMAX
+							" but could not cross-check current PID's"
+							" name: did not expect to get here",
+							(uintmax_t)pid);
+					}
+					break;
+			}
+		}
+
+		if (current_progname) {
+			free(current_progname);
+			current_progname = NULL;
+		}
+
+		/* Logged or not, sanity-check was requested and failed */
 		return -1;
 	}
+	if (current_progname) {
+		free(current_progname);
+		current_progname = NULL;
+	}
 
 	/* see if this is going to work first - does the process exist,
 	 * and do we have permissions to signal it? */
@@ -1199,6 +1301,8 @@ int sendsignalpid(pid_t pid, int sig, const char *progname)
 	NUT_UNUSED_VARIABLE(pid);
 	NUT_UNUSED_VARIABLE(sig);
 	NUT_UNUSED_VARIABLE(progname);
+	NUT_UNUSED_VARIABLE(check_current_progname);
+	/* Windows builds use named pipes, not signals per se */
 	upslogx(LOG_ERR,
 		"%s: not implemented for Win32 and "
 		"should not have been called directly!",
@@ -1239,7 +1343,7 @@ pid_t parsepid(const char *buf)
  * zero for a successfully sent signal
  */
 #ifndef WIN32
-int sendsignalfn(const char *pidfn, int sig, const char *progname)
+int sendsignalfn(const char *pidfn, int sig, const char *progname, int check_current_progname)
 {
 	char	buf[SMALLBUF];
 	FILE	*pidf;
@@ -1275,7 +1379,7 @@ int sendsignalfn(const char *pidfn, int sig, const char *progname)
 
 	if (pid >= 0) {
 		/* this method actively reports errors, if any */
-		ret = sendsignalpid(pid, sig, progname);
+		ret = sendsignalpid(pid, sig, progname, check_current_progname);
 	}
 
 	fclose(pidf);
@@ -1284,10 +1388,11 @@ int sendsignalfn(const char *pidfn, int sig, const char *progname)
 
 #else	/* => WIN32 */
 
-int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored)
+int sendsignalfn(const char *pidfn, const char * sig, const char *progname_ignored, int check_current_progname_ignored)
 {
 	BOOL	ret;
 	NUT_UNUSED_VARIABLE(progname_ignored);
+	NUT_UNUSED_VARIABLE(check_current_progname_ignored);
 
 	ret = send_to_named_pipe(pidfn, sig);
 
@@ -1350,19 +1455,21 @@ int snprintfcat(char *dst, size_t size, const char *fmt, ...)
 
 /* lazy way to send a signal if the program uses the PIDPATH */
 #ifndef WIN32
-int sendsignal(const char *progname, int sig)
+int sendsignal(const char *progname, int sig, int check_current_progname)
 {
 	char	fn[SMALLBUF];
 
 	snprintf(fn, sizeof(fn), "%s/%s.pid", rootpidpath(), progname);
 
-	return sendsignalfn(fn, sig, progname);
+	return sendsignalfn(fn, sig, progname, check_current_progname);
 }
 #else
-int sendsignal(const char *progname, const char * sig)
+int sendsignal(const char *progname, const char * sig, int check_current_progname)
 {
-	/* progname is used as the pipe name for WIN32 */
-	return sendsignalfn(progname, sig, NULL);
+	/* progname is used as the pipe name for WIN32
+	 * check_current_progname is de-facto ignored
+	 */
+	return sendsignalfn(progname, sig, NULL, check_current_progname);
 }
 #endif
 

drivers/main.c

@@ -2223,9 +2223,9 @@ int main(int argc, char **argv)
 			int cmdret = -1;
 			/* Send a signal to older copy of the driver, if any */
 			if (oldpid < 0) {
-				cmdret = sendsignalfn(pidfnbuf, cmd, progname);
+				cmdret = sendsignalfn(pidfnbuf, cmd, progname, 1);
 			} else {
-				cmdret = sendsignalpid(oldpid, cmd, progname);
+				cmdret = sendsignalpid(oldpid, cmd, progname, 1);
 			}
 
 			switch (cmdret) {
@@ -2321,7 +2321,7 @@ int main(int argc, char **argv)
 
 			upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", pidfnbuf);
 
-			if ((sigret = sendsignalfn(pidfnbuf, SIGTERM, progname) != 0)) {
+			if ((sigret = sendsignalfn(pidfnbuf, SIGTERM, progname, 1) != 0)) {
 				upsdebugx(1, "Can't send signal to PID, assume invalid PID file %s; "
 					"sendsignalfn() returned %d (errno=%d): %s",
 					pidfnbuf, sigret, errno, strerror(errno));
@@ -2339,9 +2339,9 @@ int main(int argc, char **argv)
 			struct stat	st;
 			if (stat(pidfnbuf, &st) == 0) {
 				upslogx(LOG_WARNING, "Duplicate driver instance is still alive (PID file %s exists) after several termination attempts! Killing other driver!", pidfnbuf);
-				if (sendsignalfn(pidfnbuf, SIGKILL, progname) == 0) {
+				if (sendsignalfn(pidfnbuf, SIGKILL, progname, 1) == 0) {
 					sleep(5);
-					if (sendsignalfn(pidfnbuf, 0, progname) == 0) {
+					if (sendsignalfn(pidfnbuf, 0, progname, 1) == 0) {
 						upslogx(LOG_WARNING, "Duplicate driver instance is still alive (could signal the process)");
 						/* TODO: Should we writepid() below in this case?
 						 * Or if driver init fails, restore the old content
@@ -2385,7 +2385,7 @@ int main(int argc, char **argv)
 		upslogx(LOG_WARNING, "Duplicate driver instance detected! Terminating other driver!");
 		for(i=0;i<10;i++) {
 			DWORD res;
-			sendsignal(name, COMMAND_STOP);
+			sendsignal(name, COMMAND_STOP, 1);
 			if(mutex != NULL ) {
 				res = WaitForSingleObject(mutex,1000);
 				if(res==WAIT_OBJECT_0) {