ongrep

commit 96280273f5288d9d908a4e72fdaefe9f39535ac0
parent 5f4e8ef262b0996c8b212bc4737772b52efa17ef
Author: Stephen Gregoratto <dev@sgregoratto.me>
Date:   Wed, 17 Jun 2020 13:30:27 +1000

Fix clean_exit to exit with the proper code

The manual states that an exit code of 2 is for errors. Since clean_exit
is a signal handler, passing it a value of 2 meant I was effectively
sending it SIGINT. Oops.

Fixed by passing it -1 on errors and setting the proper exit code.
Also mark the function as __dead.

Diffstat:
M
ngrep.c
 | 
59
+++++++++++++++++++++++++++++++++--------------------------
M
ngrep.h
 | 
2
+-

2 files changed, 34 insertions(+), 27 deletions(-)
diff --git a/ngrep.c b/ngrep.c
@@ -248,10 +248,8 @@ main(int argc, char **argv)
 		}
 	}
 
-	if (unveil(NULL, NULL) == -1) {
-		warn("unveil");
-		clean_exit(2);
-	}
+	if (unveil(NULL, NULL) == -1)
+		err(2, "unveil");
 
 	if (show_hex && dump_func != &dump_formatted) {
 		warnx("-x is incompatible with -W");
@@ -262,7 +260,7 @@ main(int argc, char **argv)
 
 	/* Setup PCAP input */
 	if (setup_pcap_source())
-		clean_exit(2);
+		clean_exit(-1);
 
 	/*
 	 * XXX: Originally, the filter would be setup again without the vlan
@@ -270,7 +268,7 @@ main(int argc, char **argv)
 	 */
 	if (setup_bpf_filter(argv) == -1) {
 		warnx("cannot compile filter: %s", pcap_geterr(pd));
-		clean_exit(2);
+		clean_exit(-1);
 	}
 
 	drop_privs();
@@ -283,7 +281,7 @@ main(int argc, char **argv)
 	/* Setup matcher */
 	if (match_data) {
 		if (setup_matcher())
-			clean_exit(2);
+			clean_exit(-1);
 
 		if (quiet < 2 && strlen(match_data))
 			printf("%smatch: %s%s\n", invert_match ? "don't " : "",
@@ -298,7 +296,7 @@ main(int argc, char **argv)
 		pd_dump = pcap_dump_open(pd, dump_file);
 		if (!pd_dump) {
 			warnx("pcap_dump_open: %s", pcap_geterr(pd));
-			clean_exit(2);
+			clean_exit(-2);
 		} else
 			printf("output: %s\n", dump_file);
 	}
@@ -790,7 +788,7 @@ re_match_func(unsigned char *data, uint32_t len, uint16_t *mindex,
 		case PCRE_ERROR_UNKNOWN_NODE:
 		case PCRE_ERROR_NOMEMORY:
 			warnx("pcre_exec: fatal error");
-			clean_exit(2);
+			clean_exit(-1);
 		case PCRE_ERROR_NOMATCH:
 			return 0;
 		}
@@ -969,32 +967,32 @@ get_filter_from_file(void)
 
 	if ((fd = open(filter_file, O_RDONLY)) == -1) {
 		warn("cannot open %s", filter_file);
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	if (fstat(fd, &st) == -1) {
 		warn("cannot stat %s", filter_file);
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	if (st.st_size >= SSIZE_MAX) {
 		warn("file too long: %s", filter_file);
-		clean_exit(2);
+		clean_exit(-1);
 	}
 
 	size = prefix + 8 + st.st_size + 2;
 	if (size >= SIZE_MAX)
 		warnx("fstr too long: %s", filter_file);
 	if ((fstr = malloc(size)) == NULL)
-		err(1, "malloc");
+		err(2, "malloc");
 
 	(void)strlcpy(fstr, template, size);
 	(void)strlcat(fstr, " and  ( ", size);
 	flen = read(fd, fstr + prefix, st.st_size);
 	if (flen == -1) {
 		warn("read %s", filter_file);
-		clean_exit(2);
+		clean_exit(-1);
 	} else if (flen != st.st_size) {
 		warn("short read of %s: expected %lld, got %zu", filter_file, st.st_size, flen);
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	(void)strlcat(fstr, " )", size);
 
@@ -1021,7 +1019,7 @@ get_filter_from_argv(char **argv)
 
 	if (size >= SIZE_MAX) {
 		warnx("filter too long");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 
 	if ((fstr = malloc(size)) == NULL)
@@ -1165,31 +1163,31 @@ drop_privs(void)
 
 	if ((pw = getpwnam(UNPRIV_USER)) == NULL) {
 		warn("cannot drop privileges: getpwnam");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	endpwent();
 
 	if (chroot("/var/empty") == -1) {
 		warn("cannot drop privileges: chroot");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	if (chdir("/") == -1) {
 		warn("cannot drop privileges: chdir");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 
 	gidset[0] = pw->pw_gid;
 	if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1) {
 		warn("cannot drop privileges: setresgid");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	if (setgroups(1, gidset) == -1) {
 		warn("cannot drop privileges: setgroups");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 	if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) {
 		warn("cannot drop privileges: setresuid");
-		clean_exit(2);
+		clean_exit(-1);
 	}
 
 }
@@ -1238,16 +1236,25 @@ usage(void)
 }
 
 void
-clean_exit(int32_t sig)
+clean_exit(int sig)
 {
+	int code;
+
 	signal(SIGINT, SIG_IGN);
 	signal(SIGABRT, SIG_IGN);
 	signal(SIGQUIT, SIG_IGN);
 	signal(SIGPIPE, SIG_IGN);
 	signal(SIGWINCH, SIG_IGN);
 
+	if (sig == -1)
+		code = 2;
+	else if (matches > 0)
+		code = 0;
+	else
+		code = 1;
+
 	if (quiet < 1 && sig >= 0)
-		printf("exit\n");
+		puts("exit");
 
 	if (pattern)
 		pcre_free(pattern);
@@ -1261,7 +1268,7 @@ clean_exit(int32_t sig)
 	 * "may or may not" be accurate. So useless. :-( And confusing for a
 	 * user to see counts not match what ngrep thinks.
 	 */
-	if (quiet < 1 && sig >= 0 && !read_file)
+	if (quiet < 1 && sig >= 0 && read_file == NULL)
 		printf("%u received, %u matched\n", seen_frames, matches);
 
 	if (pd)
@@ -1271,5 +1278,5 @@ clean_exit(int32_t sig)
 	if (pd_dump)
 		pcap_dump_close(pd_dump);
 
-	exit(matches ? 0 : 1);
+	exit(code);
 }
diff --git a/ngrep.h b/ngrep.h
@@ -72,7 +72,7 @@ void	process(u_char *, struct pcap_pkthdr *, u_char *);
 
 __dead void	usage(void);
 void		update_windowsize(int32_t);
-void		clean_exit(int32_t);
+__dead void	clean_exit(int);
 
 void	dump_packet(struct pcap_pkthdr *, u_char *, uint8_t, unsigned char *, uint32_t,
 		const char *, const char *, uint16_t, uint16_t, uint8_t,