MFC r273733, r273740 and r273773:

The SYSCTL data pointers can come from userspace and must not be
directly accessed. Although this will work on some platforms, it can
throw an exception if the pointer is invalid and then panic the kernel.

Add a missing SYSCTL_IN() of "SCTP_BASE_STATS" structure.

Sponsored by:	Mellanox Technologies

8 files changed, 123 insertions, 102 deletions

diff --git a/sys/dev/acpi_support/acpi_ibm.c b/sys/dev/acpi_support/acpi_ibm.c
index 3f53c59..52cee09 100644
--- a/sys/dev/acpi_support/acpi_ibm.c
+++ b/sys/dev/acpi_support/acpi_ibm.c
@@ -899,6 +899,7 @@ acpi_ibm_handlerevents_sysctl(SYSCTL_HANDLER_ARGS)
 	char			*cp, *ep;
 	int			l, val;
 	unsigned int		handler_events;
+	char			temp[128];
 
 	ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
@@ -920,17 +921,18 @@ acpi_ibm_handlerevents_sysctl(SYSCTL_HANDLER_ARGS)
 
 	sbuf_trim(&sb);
 	sbuf_finish(&sb);
-
-	/* Copy out the old values to the user. */
-	error = SYSCTL_OUT(req, sbuf_data(&sb), sbuf_len(&sb));
+	strlcpy(temp, sbuf_data(&sb), sizeof(temp));
 	sbuf_delete(&sb);
 
+	error = sysctl_handle_string(oidp, temp, sizeof(temp), req);
+
+	/* Check for error or no change */
 	if (error != 0 || req->newptr == NULL)
 		goto out;
 
 	/* If the user is setting a string, parse it. */
 	handler_events = 0;
-	cp = (char *)req->newptr;
+	cp = temp;
 	while (*cp) {
 		if (isspace(*cp)) {
 			cp++;
diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 315686b..2bf04c5 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -3748,6 +3748,7 @@ acpi_debug_sysctl(SYSCTL_HANDLER_ARGS)
     int		 error, *dbg;
     struct	 debugtag *tag;
     struct	 sbuf sb;
+    char	 temp[128];
 
     if (sbuf_new(&sb, NULL, 128, SBUF_AUTOEXTEND) == NULL)
 	return (ENOMEM);
@@ -3771,15 +3772,15 @@ acpi_debug_sysctl(SYSCTL_HANDLER_ARGS)
     }
     sbuf_trim(&sb);
     sbuf_finish(&sb);
-
-    /* Copy out the old values to the user. */
-    error = SYSCTL_OUT(req, sbuf_data(&sb), sbuf_len(&sb));
+    strlcpy(temp, sbuf_data(&sb), sizeof(temp));
     sbuf_delete(&sb);
 
-    /* If the user is setting a string, parse it. */
+    error = sysctl_handle_string(oidp, temp, sizeof(temp), req);
+
+    /* Check for error or no change */
     if (error == 0 && req->newptr != NULL) {
 	*dbg = 0;
-	setenv((char *)oidp->oid_arg1, (char *)req->newptr);
+	setenv((char *)oidp->oid_arg1, temp);
 	acpi_set_debugging(NULL);
     }
     ACPI_SERIAL_END(acpi);
diff --git a/sys/dev/asmc/asmc.c b/sys/dev/asmc/asmc.c
index 5933307..5ae4b80 100644
--- a/sys/dev/asmc/asmc.c
+++ b/sys/dev/asmc/asmc.c
@@ -1052,7 +1052,7 @@ asmc_mb_sysctl_fanminspeed(SYSCTL_HANDLER_ARGS)
 	error = sysctl_handle_int(oidp, &v, 0, req);
 
 	if (error == 0 && req->newptr != NULL) {
-		unsigned int newspeed = *(unsigned int *)req->newptr;
+		unsigned int newspeed = v;
 		asmc_fan_setvalue(dev, ASMC_KEY_FANMINSPEED, fan, newspeed);
 	}
 
@@ -1071,7 +1071,7 @@ asmc_mb_sysctl_fanmaxspeed(SYSCTL_HANDLER_ARGS)
 	error = sysctl_handle_int(oidp, &v, 0, req);
 
 	if (error == 0 && req->newptr != NULL) {
-		unsigned int newspeed = *(unsigned int *)req->newptr;
+		unsigned int newspeed = v;
 		asmc_fan_setvalue(dev, ASMC_KEY_FANMAXSPEED, fan, newspeed);
 	}
 
@@ -1090,7 +1090,7 @@ asmc_mb_sysctl_fantargetspeed(SYSCTL_HANDLER_ARGS)
 	error = sysctl_handle_int(oidp, &v, 0, req);
 
 	if (error == 0 && req->newptr != NULL) {
-		unsigned int newspeed = *(unsigned int *)req->newptr;
+		unsigned int newspeed = v;
 		asmc_fan_setvalue(dev, ASMC_KEY_FANTARGETSPEED, fan, newspeed);
 	}
 
@@ -1283,7 +1283,7 @@ asmc_mb_sysctl_sms_z(SYSCTL_HANDLER_ARGS)
 
 	asmc_sms_read(dev, ASMC_KEY_SMS_Z, &val);
 	v = (int32_t) val;
-	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
+	error = sysctl_handle_int(oidp, &v, 0, req);
 
 	return (error);
 }
@@ -1298,7 +1298,7 @@ asmc_mbp_sysctl_light_left(SYSCTL_HANDLER_ARGS)
 
 	asmc_key_read(dev, ASMC_KEY_LIGHTLEFT, buf, sizeof buf);
 	v = buf[2];
-	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
+	error = sysctl_handle_int(oidp, &v, 0, req);
 
 	return (error);
 }
@@ -1313,7 +1313,7 @@ asmc_mbp_sysctl_light_right(SYSCTL_HANDLER_ARGS)
 	
 	asmc_key_read(dev, ASMC_KEY_LIGHTRIGHT, buf, sizeof buf);
 	v = buf[2];
-	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
+	error = sysctl_handle_int(oidp, &v, 0, req);
 	
 	return (error);
 }
@@ -1324,19 +1324,19 @@ asmc_mbp_sysctl_light_control(SYSCTL_HANDLER_ARGS)
 	device_t dev = (device_t) arg1;
 	uint8_t buf[2];
 	int error;
-	unsigned int level;
-	static int32_t v;
-	
-	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
+	static unsigned int level;
+	int v;
+
+	v = level;
+	error = sysctl_handle_int(oidp, &v, 0, req);
+
 	if (error == 0 && req->newptr != NULL) {
-		level = *(unsigned int *)req->newptr;
-		if (level > 255)
+		if (v < 0 || v > 255)
 			return (EINVAL);
-		v = level;
+		level = v;
 		buf[0] = level;
 		buf[1] = 0x00;
 		asmc_key_write(dev, ASMC_KEY_LIGHTVALUE, buf, sizeof buf);
 	}
-	
 	return (error);
 }
diff --git a/sys/kern/kern_ffclock.c b/sys/kern/kern_ffclock.c
index 07441cd..c168325 100644
--- a/sys/kern/kern_ffclock.c
+++ b/sys/kern/kern_ffclock.c
@@ -203,26 +203,29 @@ static int
 sysctl_kern_sysclock_active(SYSCTL_HANDLER_ARGS)
 {
 	char newclock[MAX_SYSCLOCK_NAME_LEN];
-	int clk, error;
+	int error;
+	int clk;
+
+	/* Return the name of the current active sysclock. */
+	strlcpy(newclock, sysclocks[sysclock_active], sizeof(newclock));
+	error = sysctl_handle_string(oidp, newclock, sizeof(newclock), req);
 
-	if (req->newptr == NULL) {
-		/* Return the name of the current active sysclock. */
-		strlcpy(newclock, sysclocks[sysclock_active], sizeof(newclock));
-		error = sysctl_handle_string(oidp, newclock,
-		    sizeof(newclock), req);
-	} else {
-		/* Change the active sysclock to the user specified one. */
-		error = EINVAL;
-		for (clk = 0; clk < NUM_SYSCLOCKS; clk++) {
-			if (strncmp((char *)req->newptr, sysclocks[clk],
-			    strlen(sysclocks[clk])) == 0) {
-				sysclock_active = clk;
-				error = 0;
-				break;
-			}
+	/* Check for error or no change */
+	if (error != 0 || req->newptr == NULL)
+		goto done;
+
+	/* Change the active sysclock to the user specified one: */
+	error = EINVAL;
+	for (clk = 0; clk < NUM_SYSCLOCKS; clk++) {
+		if (strncmp(newclock, sysclocks[clk],
+		    MAX_SYSCLOCK_NAME_LEN - 1)) {
+			continue;
 		}
+		sysclock_active = clk;
+		error = 0;
+		break;
 	}
-
+done:
 	return (error);
 }
 
diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index cb3ed27..93929da 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -2755,7 +2755,8 @@ bpfstats_fill_xbpf(struct xbpf_d *d, struct bpf_d *bd)
 static int
 bpf_stats_sysctl(SYSCTL_HANDLER_ARGS)
 {
-	struct xbpf_d *xbdbuf, *xbd, zerostats;
+	static const struct xbpf_d zerostats;
+	struct xbpf_d *xbdbuf, *xbd, tempstats;
 	int index, error;
 	struct bpf_if *bp;
 	struct bpf_d *bd;
@@ -2775,11 +2776,13 @@ bpf_stats_sysctl(SYSCTL_HANDLER_ARGS)
 	 * as we aren't allowing the user to set the counters currently.
 	 */
 	if (req->newptr != NULL) {
-		if (req->newlen != sizeof(zerostats))
+		if (req->newlen != sizeof(tempstats))
 			return (EINVAL);
-		bzero(&zerostats, sizeof(zerostats));
-		xbd = req->newptr;
-		if (bcmp(xbd, &zerostats, sizeof(*xbd)) != 0)
+		memset(&tempstats, 0, sizeof(tempstats));
+		error = SYSCTL_IN(req, &tempstats, sizeof(tempstats));
+		if (error)
+			return (error);
+		if (bcmp(&tempstats, &zerostats, sizeof(tempstats)) != 0)
 			return (EINVAL);
 		bpf_zero_counters();
 		return (0);
diff --git a/sys/netinet/cc/cc.c b/sys/netinet/cc/cc.c
index 19d84ab..9f18833 100644
--- a/sys/netinet/cc/cc.c
+++ b/sys/netinet/cc/cc.c
@@ -92,33 +92,33 @@ cc_default_algo(SYSCTL_HANDLER_ARGS)
 {
 	char default_cc[TCP_CA_NAME_MAX];
 	struct cc_algo *funcs;
-	int err, found;
-
-	err = found = 0;
-
-	if (req->newptr == NULL) {
-		/* Just print the current default. */
-		CC_LIST_RLOCK();
-		strlcpy(default_cc, CC_DEFAULT()->name, TCP_CA_NAME_MAX);
-		CC_LIST_RUNLOCK();
-		err = sysctl_handle_string(oidp, default_cc, 0, req);
-	} else {
-		/* Find algo with specified name and set it to default. */
-		CC_LIST_RLOCK();
-		STAILQ_FOREACH(funcs, &cc_list, entries) {
-			if (strncmp((char *)req->newptr, funcs->name,
-			    TCP_CA_NAME_MAX) == 0) {
-				found = 1;
-				V_default_cc_ptr = funcs;
-			}
-		}
-		CC_LIST_RUNLOCK();
+	int error;
 
-		if (!found)
-			err = ESRCH;
-	}
+	/* Get the current default: */
+	CC_LIST_RLOCK();
+	strlcpy(default_cc, CC_DEFAULT()->name, sizeof(default_cc));
+	CC_LIST_RUNLOCK();
 
-	return (err);
+	error = sysctl_handle_string(oidp, default_cc, sizeof(default_cc), req);
+
+	/* Check for error or no change */
+	if (error != 0 || req->newptr == NULL)
+		goto done;
+
+	error = ESRCH;
+
+	/* Find algo with specified name and set it to default. */
+	CC_LIST_RLOCK();
+	STAILQ_FOREACH(funcs, &cc_list, entries) {
+		if (strncmp(default_cc, funcs->name, sizeof(default_cc)))
+			continue;
+		V_default_cc_ptr = funcs;
+		error = 0;
+		break;
+	}
+	CC_LIST_RUNLOCK();
+done:
+	return (error);
 }
 
 /*
diff --git a/sys/netinet/sctp_sysctl.c b/sys/netinet/sctp_sysctl.c
index 89e348a..1d97e34 100644
--- a/sys/netinet/sctp_sysctl.c
+++ b/sys/netinet/sctp_sysctl.c
@@ -636,17 +636,26 @@ sctp_sysctl_handle_stats(SYSCTL_HANDLER_ARGS)
 	int error;
 
 #if defined(SMP) && defined(SCTP_USE_PERCPU_STAT)
+	struct sctpstat sb_temp;
+	struct sctpstat *sarry;
+	struct sctpstat sb;
 	int cpu;
-	struct sctpstat sb, *sarry;
-
 #endif
 
 	if ((req->newptr != NULL) &&
 	    (req->newlen != sizeof(struct sctpstat))) {
 		return (EINVAL);
 	}
+
 #if defined(SMP) && defined(SCTP_USE_PERCPU_STAT)
-	memset(&sb, 0, sizeof(struct sctpstat));
+	memset(&sb, 0, sizeof(sb));
+	memset(&sb_temp, 0, sizeof(sb_temp));
+
+	if (req->newptr != NULL) {
+		error = SYSCTL_IN(req, &sb_temp, sizeof(sb_temp));
+		if (error != 0)
+			return (error);
+	}
 	for (cpu = 0; cpu < mp_maxid; cpu++) {
 		sarry = &SCTP_BASE_STATS[cpu];
 		if (sarry->sctps_discontinuitytime.tv_sec > sb.sctps_discontinuitytime.tv_sec) {
@@ -774,12 +783,14 @@ sctp_sysctl_handle_stats(SYSCTL_HANDLER_ARGS)
 		sb.sctps_send_burst_avoid += sarry->sctps_send_burst_avoid;
 		sb.sctps_send_cwnd_avoid += sarry->sctps_send_cwnd_avoid;
 		sb.sctps_fwdtsn_map_over += sarry->sctps_fwdtsn_map_over;
-		if (req->newptr != NULL) {
-			memcpy(sarry, req->newptr, sizeof(struct sctpstat));
-		}
+		if (req->newptr != NULL)
+			memcpy(sarry, &sb_temp, sizeof(struct sctpstat));
 	}
 	error = SYSCTL_OUT(req, &sb, sizeof(struct sctpstat));
 #else
+	error = SYSCTL_IN(req, &SCTP_BASE_STATS, sizeof(struct sctpstat));
+	if (error)
+		return (error);
 	error = SYSCTL_OUT(req, &SCTP_BASE_STATS, sizeof(struct sctpstat));
 #endif
 	return (error);
diff --git a/sys/netinet/siftr.c b/sys/netinet/siftr.c
index 886be06..a484481 100644
--- a/sys/netinet/siftr.c
+++ b/sys/netinet/siftr.c
@@ -266,6 +266,7 @@ static unsigned int siftr_pkts_per_log = 1;
 static unsigned int siftr_generate_hashes = 0;
 /* static unsigned int siftr_binary_log = 0; */
 static char siftr_logfile[PATH_MAX] = "/var/log/siftr.log";
+static char siftr_logfile_shadow[PATH_MAX] = "/var/log/siftr.log";
 static u_long siftr_hashmask;
 STAILQ_HEAD(pkthead, pkt_node) pkt_queue = STAILQ_HEAD_INITIALIZER(pkt_queue);
 LIST_HEAD(listhead, flow_hash_node) *counter_hash;
@@ -297,7 +298,7 @@ SYSCTL_PROC(_net_inet_siftr, OID_AUTO, enabled, CTLTYPE_UINT|CTLFLAG_RW,
     "switch siftr module operations on/off");
 
 SYSCTL_PROC(_net_inet_siftr, OID_AUTO, logfile, CTLTYPE_STRING|CTLFLAG_RW,
-    &siftr_logfile, sizeof(siftr_logfile), &siftr_sysctl_logfile_name_handler,
+    &siftr_logfile_shadow, sizeof(siftr_logfile_shadow), &siftr_sysctl_logfile_name_handler,
     "A", "file to save siftr log messages to");
 
 SYSCTL_UINT(_net_inet_siftr, OID_AUTO, ppl, CTLFLAG_RW,
@@ -1143,38 +1144,38 @@ siftr_sysctl_logfile_name_handler(SYSCTL_HANDLER_ARGS)
 	struct alq *new_alq;
 	int error;
 
-	if (req->newptr == NULL)
-		goto skip;
+	error = sysctl_handle_string(oidp, arg1, arg2, req);
 
-	/* If old filename and new filename are different. */
-	if (strncmp(siftr_logfile, (char *)req->newptr, PATH_MAX)) {
-
-		error = alq_open(&new_alq, req->newptr, curthread->td_ucred,
-		    SIFTR_LOG_FILE_MODE, SIFTR_ALQ_BUFLEN, 0);
+	/* Check for error or same filename */
+	if (error != 0 || req->newptr == NULL ||
+	    strncmp(siftr_logfile, arg1, arg2) == 0)
+		goto done;
 
-		/* Bail if unable to create new alq. */
-		if (error)
-			return (1);
+	/* Filname changed */
+	error = alq_open(&new_alq, arg1, curthread->td_ucred,
+	    SIFTR_LOG_FILE_MODE, SIFTR_ALQ_BUFLEN, 0);
+	if (error != 0)
+		goto done;
 
-		/*
-		 * If disabled, siftr_alq == NULL so we simply close
-		 * the alq as we've proved it can be opened.
-		 * If enabled, close the existing alq and switch the old
-		 * for the new.
-		 */
-		if (siftr_alq == NULL)
-			alq_close(new_alq);
-		else {
-			alq_close(siftr_alq);
-			siftr_alq = new_alq;
-		}
+	/*
+	 * If disabled, siftr_alq == NULL so we simply close
+	 * the alq as we've proved it can be opened.
+	 * If enabled, close the existing alq and switch the old
+	 * for the new.
+	 */
+	if (siftr_alq == NULL) {
+		alq_close(new_alq);
+	} else {
+		alq_close(siftr_alq);
+		siftr_alq = new_alq;
 	}
 
-skip:
-	return (sysctl_handle_string(oidp, arg1, arg2, req));
+	/* Update filename upon success */
+	strlcpy(siftr_logfile, arg1, arg2);
+done:
+	return (error);
 }
 
-
 static int
 siftr_manage_ops(uint8_t action)
 {