Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 14 Nov 2020 01:28:04 +0000 (UTC)
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r367657 - in stable/12: sys/kern sys/sys usr.sbin/binmiscctl
Message-ID:  <202011140128.0AE1S4WH076368@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kevans
Date: Sat Nov 14 01:28:04 2020
New Revision: 367657
URL: https://svnweb.freebsd.org/changeset/base/367657

Log:
  MFC imgact_binmisc housecleaning: r367361, r367439, r367441-r367442,
  r367444, r367452, r367456, r367477
  
  r367361:
  imgact_binmisc: fix up some minor nits
  
  - Removed a bunch of redundant headers
  - Don't explicitly initialize to 0
  - The !error check prior to setting imgp->interpreter_name is redundant, all
    error paths should and do return or go to 'done'. We have larger problems
    otherwise.
  
  r367439:
  imgact_binmisc: minor re-organization of imgact_binmisc_exec exits
  
  Notably, streamline error paths through the existing 'done' label, making it
  easier to quickly verify correct cleanup.
  
  Future work might add a kernel-only flag to indicate that a interpreter uses
  #a. Currently, all executions via imgact_binmisc pay the penalty of
  constructing sname/fname, even if they will not use it. qemu-user-static
  doesn't need it, the stock rc script for qemu-user-static certainly doesn't
  use it, and I suspect these are the vast majority of (if not the only)
  current users.
  
  r367441:
  binmiscctl(8): miscellaneous cleanup
  
  - Bad whitespace in Makefile.
  - Reordered headers, sys/ first.
  - Annotated fatal/usage __dead2 to help `make analyze` out a little bit.
  - Spell a couple of sizeof constructs as "nitems" and "howmany" instead.
  
  r367442:
  imgact_binmisc: validate flags coming from userland
  
  We may want to reserve bits in the future for kernel-only use, so start
  rejecting any that aren't the two that we're currently expecting from
  userland.
  
  r367444:
  imgact_binmisc: abstract away the list lock (NFC)
  
  This module handles relatively few execs (initial qemu-user-static, then
  qemu-user-static handles exec'ing itself for binaries it's already running),
  but all execs pay the price of at least taking the relatively expensive
  sx/slock to check for a match when this module is loaded. Future work will
  almost certainly swap this out for another lock, perhaps an rmslock.
  
  The RLOCK/WLOCK phrasing was chosen based on what the callers are really
  wanting, rather than using the verbiage typically appropriate for an sx.
  
  r367452:
  imgact_binmisc: reorder members of struct imgact_binmisc_entry (NFC)
  
  This doesn't change anything at the moment since the out-of-order elements
  were a pair of uint32_t, but future additions may have caused unnecessary
  padding by following the existing precedent.
  
  r367456:
  imgact_binmisc: move some calculations out of the exec path
  
  The offset we need to account for in the interpreter string comes in two
  variants:
  
  1. Fixed - macros other than #a that will not vary from invocation to
     invocation
  2. Variable - #a, which is substitued with the argv0 that we're replacing
  
  Note that we don't have a mechanism to modify an existing entry.  By
  recording both of these offset requirements when the interpreter is added,
  we can avoid some unnecessary calculations in the exec path.
  
  Most importantly, we can know up-front whether we need to grab
  calculate/grab the the filename for this interpreter. We also get to avoid
  walking the string a first time looking for macros. For most invocations,
  it's a swift exit as they won't have any, but there's no point entering a
  loop and searching for the macro indicator if we already know there will not
  be one.
  
  While we're here, go ahead and only calculate the argv0 name length once per
  invocation. While it's unlikely that we'll have more than one #a, there's no
  reason to recalculate it every time we encounter an #a when it will not
  change.
  
  I have not bothered trying to benchmark this at all, because it's arguably a
  minor and straightforward/obvious improvement.
  
  r367477:
  imgact_binmisc: limit the extent of match on incoming entries
  
  imgact_binmisc matches magic/mask from imgp->image_header, which is only a
  single page in size mapped from the first page of an image. One can specify
  an interpreter that matches on, e.g., --offset 4096 --size 256 to read up to
  256 bytes past the mapped first page.
  
  The limitation is that we cannot specify a magic string that exceeds a
  single page, and we can't allow offset + size to exceed a single page
  either.  A static assert has been added in case someone finds it useful to
  try and expand the size, but it does seem a little unlikely.
  
  While this looks kind of exploitable at a sideways squinty-glance, there are
  a couple of mitigating factors:
  
  1.) imgact_binmisc is not enabled by default,
  2.) entries may only be added by the superuser,
  3.) trying to exploit this information to read what's mapped past the end
    would be worse than a root canal or some other relatably painful
    experience, and
  4.) there's no way one could pull this off without it being completely
    obvious.
  
  The first page is mapped out of an sf_buf, the implementation of which (or
  lack thereof) depends on your platform.

Modified:
  stable/12/sys/kern/imgact_binmisc.c
  stable/12/sys/sys/imgact_binmisc.h
  stable/12/usr.sbin/binmiscctl/Makefile
  stable/12/usr.sbin/binmiscctl/binmiscctl.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/kern/imgact_binmisc.c
==============================================================================
--- stable/12/sys/kern/imgact_binmisc.c	Sat Nov 14 00:59:55 2020	(r367656)
+++ stable/12/sys/kern/imgact_binmisc.c	Sat Nov 14 01:28:04 2020	(r367657)
@@ -29,17 +29,14 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
 #include <sys/ctype.h>
-#include <sys/sbuf.h>
-#include <sys/systm.h>
-#include <sys/sysproto.h>
 #include <sys/exec.h>
 #include <sys/imgact.h>
 #include <sys/imgact_binmisc.h>
 #include <sys/kernel.h>
-#include <sys/libkern.h>
 #include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/mutex.h>
+#include <sys/sbuf.h>
 #include <sys/sysctl.h>
 #include <sys/sx.h>
 
@@ -61,16 +58,18 @@ __FBSDID("$FreeBSD$");
  * Node of the interpreter list.
  */
 typedef struct imgact_binmisc_entry {
+	SLIST_ENTRY(imgact_binmisc_entry) link;
 	char				 *ibe_name;
 	uint8_t				 *ibe_magic;
-	uint32_t			  ibe_moffset;
-	uint32_t			  ibe_msize;
 	uint8_t				 *ibe_mask;
 	uint8_t				 *ibe_interpreter;
+	ssize_t				  ibe_interp_offset;
 	uint32_t			  ibe_interp_argcnt;
 	uint32_t			  ibe_interp_length;
+	uint32_t			  ibe_argv0_cnt;
 	uint32_t			  ibe_flags;
-	SLIST_ENTRY(imgact_binmisc_entry) link;
+	uint32_t			  ibe_moffset;
+	uint32_t			  ibe_msize;
 } imgact_binmisc_entry_t;
 
 /*
@@ -97,10 +96,20 @@ MALLOC_DEFINE(M_BINMISC, KMOD_NAME, "misc binary image
 static SLIST_HEAD(, imgact_binmisc_entry) interpreter_list =
 	SLIST_HEAD_INITIALIZER(interpreter_list);
 
-static int interp_list_entry_count = 0;
+static int interp_list_entry_count;
 
 static struct sx interp_list_sx;
 
+#define	INTERP_LIST_WLOCK()		sx_xlock(&interp_list_sx)
+#define	INTERP_LIST_RLOCK()		sx_slock(&interp_list_sx)
+#define	INTERP_LIST_WUNLOCK()		sx_xunlock(&interp_list_sx)
+#define	INTERP_LIST_RUNLOCK()		sx_sunlock(&interp_list_sx)
+
+#define	INTERP_LIST_LOCK_INIT()		sx_init(&interp_list_sx, KMOD_NAME)
+#define	INTERP_LIST_LOCK_DESTROY()	sx_destroy(&interp_list_sx)
+
+#define	INTERP_LIST_ASSERT_LOCKED()	sx_assert(&interp_list_sx, SA_LOCKED)
+
 /*
  * Populate the entry with the information about the interpreter.
  */
@@ -147,7 +156,8 @@ imgact_binmisc_populate_interp(char *str, imgact_binmi
  * Allocate memory and populate a new entry for the interpreter table.
  */
 static imgact_binmisc_entry_t *
-imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe)
+imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe, ssize_t interp_offset,
+    int argv0_cnt)
 {
 	imgact_binmisc_entry_t *ibe = NULL;
 	size_t namesz = min(strlen(xbe->xbe_name) + 1, IBE_NAME_MAX);
@@ -168,7 +178,8 @@ imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe)
 	ibe->ibe_moffset = xbe->xbe_moffset;
 	ibe->ibe_msize = xbe->xbe_msize;
 	ibe->ibe_flags = xbe->xbe_flags;
-
+	ibe->ibe_interp_offset = interp_offset;
+	ibe->ibe_argv0_cnt = argv0_cnt;
 	return (ibe);
 }
 
@@ -201,7 +212,7 @@ imgact_binmisc_find_entry(char *name)
 {
 	imgact_binmisc_entry_t *ibe;
 
-	sx_assert(&interp_list_sx, SA_LOCKED);
+	INTERP_LIST_ASSERT_LOCKED();
 
 	SLIST_FOREACH(ibe, &interpreter_list, link) {
 		if (strncmp(name, ibe->ibe_name, IBE_NAME_MAX) == 0)
@@ -220,10 +231,13 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
 {
 	imgact_binmisc_entry_t *ibe;
 	char *p;
-	int cnt;
+	ssize_t interp_offset;
+	int argv0_cnt, cnt;
 
 	if (xbe->xbe_msize > IBE_MAGIC_MAX)
 		return (EINVAL);
+	if (xbe->xbe_moffset + xbe->xbe_msize > IBE_MATCH_MAX)
+		return (EINVAL);
 
 	for(cnt = 0, p = xbe->xbe_name; *p != 0; cnt++, p++)
 		if (cnt >= IBE_NAME_MAX || !isascii((int)*p))
@@ -235,23 +249,21 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
 
 	/* Make sure we don't have any invalid #'s. */
 	p = xbe->xbe_interpreter;
-	while (1) {
-		p = strchr(p, '#');
-		if (!p)
-			break;
-
+	interp_offset = 0;
+	argv0_cnt = 0;
+	while ((p = strchr(p, '#')) != NULL) {
 		p++;
 		switch(*p) {
 		case ISM_POUND:
 			/* "##" */
 			p++;
+			interp_offset--;
 			break;
-
 		case ISM_OLD_ARGV0:
 			/* "#a" */
 			p++;
+			argv0_cnt++;
 			break;
-
 		case 0:
 		default:
 			/* Anything besides the above is invalid. */
@@ -259,18 +271,18 @@ imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
 		}
 	}
 
-	sx_xlock(&interp_list_sx);
+	INTERP_LIST_WLOCK();
 	if (imgact_binmisc_find_entry(xbe->xbe_name) != NULL) {
-		sx_xunlock(&interp_list_sx);
+		INTERP_LIST_WUNLOCK();
 		return (EEXIST);
 	}
 
 	/* Preallocate a new entry. */
-	ibe = imgact_binmisc_new_entry(xbe);
+	ibe = imgact_binmisc_new_entry(xbe, interp_offset, argv0_cnt);
 
 	SLIST_INSERT_HEAD(&interpreter_list, ibe, link);
 	interp_list_entry_count++;
-	sx_xunlock(&interp_list_sx);
+	INTERP_LIST_WUNLOCK();
 
 	return (0);
 }
@@ -284,14 +296,14 @@ imgact_binmisc_remove_entry(char *name)
 {
 	imgact_binmisc_entry_t *ibe;
 
-	sx_xlock(&interp_list_sx);
+	INTERP_LIST_WLOCK();
 	if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-		sx_xunlock(&interp_list_sx);
+		INTERP_LIST_WUNLOCK();
 		return (ENOENT);
 	}
 	SLIST_REMOVE(&interpreter_list, ibe, imgact_binmisc_entry, link);
 	interp_list_entry_count--;
-	sx_xunlock(&interp_list_sx);
+	INTERP_LIST_WUNLOCK();
 
 	imgact_binmisc_destroy_entry(ibe);
 
@@ -307,14 +319,14 @@ imgact_binmisc_disable_entry(char *name)
 {
 	imgact_binmisc_entry_t *ibe;
 
-	sx_xlock(&interp_list_sx);
+	INTERP_LIST_WLOCK();
 	if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-		sx_xunlock(&interp_list_sx);
+		INTERP_LIST_WUNLOCK();
 		return (ENOENT);
 	}
 
 	ibe->ibe_flags &= ~IBF_ENABLED;
-	sx_xunlock(&interp_list_sx);
+	INTERP_LIST_WUNLOCK();
 
 	return (0);
 }
@@ -328,14 +340,14 @@ imgact_binmisc_enable_entry(char *name)
 {
 	imgact_binmisc_entry_t *ibe;
 
-	sx_xlock(&interp_list_sx);
+	INTERP_LIST_WLOCK();
 	if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-		sx_xunlock(&interp_list_sx);
+		INTERP_LIST_WUNLOCK();
 		return (ENOENT);
 	}
 
 	ibe->ibe_flags |= IBF_ENABLED;
-	sx_xunlock(&interp_list_sx);
+	INTERP_LIST_WUNLOCK();
 
 	return (0);
 }
@@ -346,8 +358,7 @@ imgact_binmisc_populate_xbe(ximgact_binmisc_entry_t *x
 {
 	uint32_t i;
 
-	sx_assert(&interp_list_sx, SA_LOCKED);
-
+	INTERP_LIST_ASSERT_LOCKED();
 	memset(xbe, 0, sizeof(*xbe));
 	strlcpy(xbe->xbe_name, ibe->ibe_name, IBE_NAME_MAX);
 
@@ -378,14 +389,14 @@ imgact_binmisc_lookup_entry(char *name, ximgact_binmis
 	imgact_binmisc_entry_t *ibe;
 	int error = 0;
 
-	sx_slock(&interp_list_sx);
+	INTERP_LIST_RLOCK();
 	if ((ibe = imgact_binmisc_find_entry(name)) == NULL) {
-		sx_sunlock(&interp_list_sx);
+		INTERP_LIST_RUNLOCK();
 		return (ENOENT);
 	}
 
 	error = imgact_binmisc_populate_xbe(xbe, ibe);
-	sx_sunlock(&interp_list_sx);
+	INTERP_LIST_RUNLOCK();
 
 	return (error);
 }
@@ -400,7 +411,7 @@ imgact_binmisc_get_all_entries(struct sysctl_req *req)
 	imgact_binmisc_entry_t *ibe;
 	int error = 0, count;
 
-	sx_slock(&interp_list_sx);
+	INTERP_LIST_RLOCK();
 	count = interp_list_entry_count;
 	xbe = malloc(sizeof(*xbe) * count, M_BINMISC, M_WAITOK|M_ZERO);
 
@@ -410,7 +421,7 @@ imgact_binmisc_get_all_entries(struct sysctl_req *req)
 		if (error)
 			break;
 	}
-	sx_sunlock(&interp_list_sx);
+	INTERP_LIST_RUNLOCK();
 
 	if (!error)
 		error = SYSCTL_OUT(req, xbe, sizeof(*xbe) * count);
@@ -437,6 +448,8 @@ sysctl_kern_binmisc(SYSCTL_HANDLER_ARGS)
 			return (error);
 		if (IBE_VERSION != xbe.xbe_version)
 			return (EINVAL);
+		if ((xbe.xbe_flags & ~IBF_VALID_UFLAGS) != 0)
+			return (EINVAL);
 		if (interp_list_entry_count == IBE_MAX_ENTRIES)
 			return (ENOSPC);
 		error = imgact_binmisc_add_entry(&xbe);
@@ -547,7 +560,7 @@ imgact_binmisc_find_interpreter(const char *image_head
 	int i;
 	size_t sz;
 
-	sx_assert(&interp_list_sx, SA_LOCKED);
+	INTERP_LIST_ASSERT_LOCKED();
 
 	SLIST_FOREACH(ibe, &interpreter_list, link) {
 		if (!(IBF_ENABLED & ibe->ibe_flags))
@@ -578,35 +591,47 @@ imgact_binmisc_exec(struct image_params *imgp)
 	const char *image_header = imgp->image_header;
 	const char *fname = NULL;
 	int error = 0;
-	size_t offset, l;
+#ifdef INVARIANTS
+	int argv0_cnt = 0;
+#endif
+	size_t namelen, offset;
 	imgact_binmisc_entry_t *ibe;
 	struct sbuf *sname;
 	char *s, *d;
 
+	sname = NULL;
+	namelen = 0;
 	/* Do we have an interpreter for the given image header? */
-	sx_slock(&interp_list_sx);
+	INTERP_LIST_RLOCK();
 	if ((ibe = imgact_binmisc_find_interpreter(image_header)) == NULL) {
-		sx_sunlock(&interp_list_sx);
-		return (-1);
+		error = -1;
+		goto done;
 	}
 
 	/* No interpreter nesting allowed. */
 	if (imgp->interpreted & IMGACT_BINMISC) {
-		sx_sunlock(&interp_list_sx);
-		return (ENOEXEC);
+		error = ENOEXEC;
+		goto done;
 	}
 
 	imgp->interpreted |= IMGACT_BINMISC;
 
-	if (imgp->args->fname != NULL) {
-		fname = imgp->args->fname;
-		sname = NULL;
-	} else {
-		/* Use the fdescfs(5) path for fexecve(2). */
-		sname = sbuf_new_auto();
-		sbuf_printf(sname, "/dev/fd/%d", imgp->args->fd);
-		sbuf_finish(sname);
-		fname = sbuf_data(sname);
+	/*
+	 * Don't bother with the overhead of putting fname together if we're not
+	 * using #a.
+	 */
+	if (ibe->ibe_argv0_cnt != 0) {
+		if (imgp->args->fname != NULL) {
+			fname = imgp->args->fname;
+		} else {
+			/* Use the fdescfs(5) path for fexecve(2). */
+			sname = sbuf_new_auto();
+			sbuf_printf(sname, "/dev/fd/%d", imgp->args->fd);
+			sbuf_finish(sname);
+			fname = sbuf_data(sname);
+		}
+
+		namelen = strlen(fname);
 	}
 
 
@@ -615,43 +640,17 @@ imgact_binmisc_exec(struct image_params *imgp)
 	 * we first shift all the other values in the `begin_argv' area to
 	 * provide the exact amount of room for the values added.  Set up
 	 * `offset' as the number of bytes to be added to the `begin_argv'
-	 * area.
+	 * area.  ibe_interp_offset is the fixed offset from macros present in
+	 * the interpreter string.
 	 */
-	offset = ibe->ibe_interp_length;
+	offset = ibe->ibe_interp_length + ibe->ibe_interp_offset;
 
-	/* Adjust the offset for #'s. */
-	s = ibe->ibe_interpreter;
-	while (1) {
-		s = strchr(s, '#');
-		if (!s)
-			break;
+	/* Variable offset to be added from macros to the interpreter string. */
+	MPASS(ibe->ibe_argv0_cnt == 0 || namelen > 0);
+	offset += ibe->ibe_argv0_cnt * (namelen - 2);
 
-		s++;
-		switch(*s) {
-		case ISM_POUND:
-			/* "##" -> "#": reduce offset by one. */
-			offset--;
-			break;
-
-		case ISM_OLD_ARGV0:
-			/* "#a" -> (old argv0): increase offset to fit fname */
-			offset += strlen(fname) - 2;
-			break;
-
-		default:
-			/* Hmm... This shouldn't happen. */
-			sx_sunlock(&interp_list_sx);
-			printf("%s: Unknown macro #%c sequence in "
-			    "interpreter string\n", KMOD_NAME, *(s + 1));
-			error = EINVAL;
-			goto done;
-		}
-		s++;
-	}
-
 	/* Check to make sure we won't overrun the stringspace. */
 	if (offset > imgp->args->stringspace) {
-		sx_sunlock(&interp_list_sx);
 		error = E2BIG;
 		goto done;
 	}
@@ -684,26 +683,20 @@ imgact_binmisc_exec(struct image_params *imgp)
 				/* "##": Replace with a single '#' */
 				*d++ = '#';
 				break;
-
 			case ISM_OLD_ARGV0:
 				/* "#a": Replace with old arg0 (fname). */
-				if ((l = strlen(fname)) != 0) {
-					memcpy(d, fname, l);
-					d += l;
-				}
+				MPASS(ibe->ibe_argv0_cnt >= ++argv0_cnt);
+				memcpy(d, fname, namelen);
+				d += namelen;
 				break;
-
 			default:
-				/* Shouldn't happen but skip it if it does. */
-				break;
+				__assert_unreachable();
 			}
 			break;
-
 		case ' ':
 			/* Replace space with NUL to separate arguments. */
 			*d++ = '\0';
 			break;
-
 		default:
 			*d++ = *s;
 			break;
@@ -711,13 +704,14 @@ imgact_binmisc_exec(struct image_params *imgp)
 		s++;
 	}
 	*d = '\0';
-	sx_sunlock(&interp_list_sx);
 
-	if (!error)
-		imgp->interpreter_name = imgp->args->begin_argv;
+	/* Catch ibe->ibe_argv0_cnt counting more #a than we did. */
+	MPASS(ibe->ibe_argv0_cnt == argv0_cnt);
+	imgp->interpreter_name = imgp->args->begin_argv;
 
 
 done:
+	INTERP_LIST_RUNLOCK();
 	if (sname)
 		sbuf_delete(sname);
 	return (error);
@@ -727,7 +721,7 @@ static void
 imgact_binmisc_init(void *arg)
 {
 
-	sx_init(&interp_list_sx, KMOD_NAME);
+	INTERP_LIST_LOCK_INIT();
 }
 
 static void
@@ -736,15 +730,15 @@ imgact_binmisc_fini(void *arg)
 	imgact_binmisc_entry_t *ibe, *ibe_tmp;
 
 	/* Free all the interpreters. */
-	sx_xlock(&interp_list_sx);
+	INTERP_LIST_WLOCK();
 	SLIST_FOREACH_SAFE(ibe, &interpreter_list, link, ibe_tmp) {
 		SLIST_REMOVE(&interpreter_list, ibe, imgact_binmisc_entry,
 		    link);
 		imgact_binmisc_destroy_entry(ibe);
 	}
-	sx_xunlock(&interp_list_sx);
+	INTERP_LIST_WUNLOCK();
 
-	sx_destroy(&interp_list_sx);
+	INTERP_LIST_LOCK_DESTROY();
 }
 
 SYSINIT(imgact_binmisc, SI_SUB_EXEC, SI_ORDER_MIDDLE, imgact_binmisc_init,

Modified: stable/12/sys/sys/imgact_binmisc.h
==============================================================================
--- stable/12/sys/sys/imgact_binmisc.h	Sat Nov 14 00:59:55 2020	(r367656)
+++ stable/12/sys/sys/imgact_binmisc.h	Sat Nov 14 01:28:04 2020	(r367657)
@@ -47,11 +47,18 @@
 #define	IBE_INTERP_LEN_MAX	(MAXPATHLEN + IBE_ARG_LEN_MAX)
 #define	IBE_MAX_ENTRIES	64	/* Max number of interpreter entries. */
 
+/* We only map the first page for identification purposes. */
+#define	IBE_MATCH_MAX	PAGE_SIZE
+_Static_assert(IBE_MAGIC_MAX <= IBE_MATCH_MAX,
+    "Cannot identify binaries past the first page.");
+
 /*
  * Imgact bin misc interpreter entry flags.
  */
 #define	IBF_ENABLED	0x0001	/* Entry is active. */
 #define	IBF_USE_MASK	0x0002	/* Use mask on header magic field. */
+
+#define	IBF_VALID_UFLAGS	0x0003	/* Bits allowed from userland. */
 
 /*
  * Used with sysctlbyname() to pass imgact bin misc entries in and out of the

Modified: stable/12/usr.sbin/binmiscctl/Makefile
==============================================================================
--- stable/12/usr.sbin/binmiscctl/Makefile	Sat Nov 14 00:59:55 2020	(r367656)
+++ stable/12/usr.sbin/binmiscctl/Makefile	Sat Nov 14 01:28:04 2020	(r367657)
@@ -3,7 +3,7 @@
 #
 
 .include <bsd.own.mk>
-	
+
 PROG=	binmiscctl
 MAN=	binmiscctl.8
 

Modified: stable/12/usr.sbin/binmiscctl/binmiscctl.c
==============================================================================
--- stable/12/usr.sbin/binmiscctl/binmiscctl.c	Sat Nov 14 00:59:55 2020	(r367656)
+++ stable/12/usr.sbin/binmiscctl/binmiscctl.c	Sat Nov 14 01:28:04 2020	(r367657)
@@ -28,6 +28,12 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 
+#include <sys/types.h>
+#include <sys/imgact_binmisc.h>
+#include <sys/linker.h>
+#include <sys/module.h>
+#include <sys/sysctl.h>
+
 #include <ctype.h>
 #include <errno.h>
 #include <getopt.h>
@@ -37,12 +43,6 @@ __FBSDID("$FreeBSD$");
 #include <stdlib.h>
 #include <string.h>
 
-#include <sys/types.h>
-#include <sys/imgact_binmisc.h>
-#include <sys/linker.h>
-#include <sys/module.h>
-#include <sys/sysctl.h>
-
 enum cmd {
 	CMD_ADD = 0,
 	CMD_REMOVE,
@@ -134,7 +134,7 @@ static char const *cmd_sysctl_name[] = {
 	IBE_SYSCTL_NAME_LIST
 };
 
-static void
+static void __dead2
 usage(const char *format, ...)
 {
 	va_list args;
@@ -150,7 +150,7 @@ usage(const char *format, ...)
 	fprintf(stderr, "\n");
 	fprintf(stderr, "usage: %s command [args...]\n\n", __progname);
 
-	for(i = 0; i < ( sizeof (cmds) / sizeof (cmds[0])); i++) {
+	for(i = 0; i < nitems(cmds); i++) {
 		fprintf(stderr, "%s:\n", cmds[i].desc);
 		fprintf(stderr, "\t%s %s %s\n\n", __progname, cmds[i].name,
 		    cmds[i].args);
@@ -159,7 +159,7 @@ usage(const char *format, ...)
 	exit (error);
 }
 
-static void
+static void __dead2
 fatal(const char *format, ...)
 {
 	va_list args;
@@ -232,7 +232,7 @@ demux_cmd(__unused int argc, char *const argv[])
 	optind = 1;
 	optreset = 1;
 
-	for(i = 0; i < ( sizeof (cmds) / sizeof (cmds[0])); i++) {
+	for(i = 0; i < nitems(cmds); i++) {
 		if (!strcasecmp(cmds[i].name, argv[0])) {
 			return (i);
 		}
@@ -505,7 +505,7 @@ main(int argc, char **argv)
 			free(xbe_outp);
 			fatal("Fatal: %s", strerror(errno));
 		}
-		for(i = 0; i < (xbe_out_sz / sizeof(xbe_out)); i++)
+		for(i = 0; i < howmany(xbe_out_sz, sizeof(xbe_out)); i++)
 			printxbe(&xbe_outp[i]);
 	}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202011140128.0AE1S4WH076368>