Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 31 Oct 2014 18:53:17 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r273916 - in stable/9/sys: dev/acpi_support dev/acpica dev/asmc net netinet netinet/cc
Message-ID:  <201410311853.s9VIrHml092912@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Fri Oct 31 18:53:16 2014
New Revision: 273916
URL: https://svnweb.freebsd.org/changeset/base/273916

Log:
  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.
  
  Sponsored by:	Mellanox Technologies

Modified:
  stable/9/sys/dev/acpi_support/acpi_ibm.c
  stable/9/sys/dev/acpica/acpi.c
  stable/9/sys/dev/asmc/asmc.c
  stable/9/sys/net/bpf.c
  stable/9/sys/netinet/cc/cc.c
  stable/9/sys/netinet/sctp_sysctl.c
  stable/9/sys/netinet/siftr.c
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/dev/   (props changed)
  stable/9/sys/net/   (props changed)

Modified: stable/9/sys/dev/acpi_support/acpi_ibm.c
==============================================================================
--- stable/9/sys/dev/acpi_support/acpi_ibm.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/dev/acpi_support/acpi_ibm.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -899,6 +899,7 @@ acpi_ibm_handlerevents_sysctl(SYSCTL_HAN
 	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_HAN
 
 	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++;

Modified: stable/9/sys/dev/acpica/acpi.c
==============================================================================
--- stable/9/sys/dev/acpica/acpi.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/dev/acpica/acpi.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -3744,6 +3744,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);
@@ -3767,15 +3768,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);

Modified: stable/9/sys/dev/asmc/asmc.c
==============================================================================
--- stable/9/sys/dev/asmc/asmc.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/dev/asmc/asmc.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -1052,7 +1052,7 @@ asmc_mb_sysctl_fanminspeed(SYSCTL_HANDLE
 	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_HANDLE
 	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_HAN
 	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_HANDLE
 
 	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_HANDL
 	
 	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_HAN
 	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);
 }

Modified: stable/9/sys/net/bpf.c
==============================================================================
--- stable/9/sys/net/bpf.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/net/bpf.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -2758,7 +2758,8 @@ bpfstats_fill_xbpf(struct xbpf_d *d, str
 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;
@@ -2778,11 +2779,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);

Modified: stable/9/sys/netinet/cc/cc.c
==============================================================================
--- stable/9/sys/netinet/cc/cc.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/netinet/cc/cc.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -92,33 +92,33 @@ cc_default_algo(SYSCTL_HANDLER_ARGS)
 {
 	char default_cc[TCP_CA_NAME_MAX];
 	struct cc_algo *funcs;
-	int err, found;
+	int error;
 
-	err = found = 0;
+	/* Get the current default: */
+	CC_LIST_RLOCK();
+	strlcpy(default_cc, CC_DEFAULT()->name, sizeof(default_cc));
+	CC_LIST_RUNLOCK();
 
-	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();
+	error = sysctl_handle_string(oidp, default_cc, sizeof(default_cc), req);
 
-		if (!found)
-			err = ESRCH;
-	}
+	/* Check for error or no change */
+	if (error != 0 || req->newptr == NULL)
+		goto done;
 
-	return (err);
+	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);
 }
 
 /*

Modified: stable/9/sys/netinet/sctp_sysctl.c
==============================================================================
--- stable/9/sys/netinet/sctp_sysctl.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/netinet/sctp_sysctl.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -684,14 +684,18 @@ static int
 sysctl_stat_get(SYSCTL_HANDLER_ARGS)
 {
 	int cpu, error;
-	struct sctpstat sb, *sarry, *cpin = NULL;
+	struct sctpstat sb, sb_temp, *sarry, *cpin = NULL;
 
 	if ((req->newptr) && (req->newlen == sizeof(struct sctpstat))) {
 		/*
 		 * User wants us to clear or at least reset the counters to
 		 * the specified values.
 		 */
-		cpin = (struct sctpstat *)req->newptr;
+		cpin = &sb_temp;
+		memset(&sb_temp, 0, sizeof(sb_temp));
+		error = SYSCTL_IN(req, &sb_temp, sizeof(sb_temp));
+		if (error != 0)
+			return (error);
 	} else if (req->newptr) {
 		/* Must be a stat structure */
 		return (EINVAL);

Modified: stable/9/sys/netinet/siftr.c
==============================================================================
--- stable/9/sys/netinet/siftr.c	Fri Oct 31 18:42:56 2014	(r273915)
+++ stable/9/sys/netinet/siftr.c	Fri Oct 31 18:53:16 2014	(r273916)
@@ -266,6 +266,7 @@ static unsigned int siftr_pkts_per_log =
 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, e
     "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,
@@ -1142,38 +1143,38 @@ siftr_sysctl_logfile_name_handler(SYSCTL
 	struct alq *new_alq;
 	int error;
 
-	if (req->newptr == NULL)
-		goto skip;
-
-	/* 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);
+	error = sysctl_handle_string(oidp, arg1, arg2, req);
 
-		/* Bail if unable to create new alq. */
-		if (error)
-			return (1);
+	/* Check for error or same filename */
+	if (error != 0 || req->newptr == NULL ||
+	    strncmp(siftr_logfile, arg1, arg2) == 0)
+		goto done;
+
+	/* 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)
 {



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