Author: hselasky
Date: Tue Oct 28 12:00:39 2014
New Revision: 273773
URL: https://svnweb.freebsd.org/changeset/base/273773

Log:
  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.
  
  MFC after:    3 days
  Sponsored by: Mellanox Technologies

Modified:
  head/sys/dev/acpi_support/acpi_ibm.c
  head/sys/dev/acpica/acpi.c
  head/sys/dev/asmc/asmc.c
  head/sys/kern/kern_ffclock.c
  head/sys/net/bpf.c
  head/sys/netinet/cc/cc.c
  head/sys/netinet/sctp_sysctl.c
  head/sys/netinet/siftr.c

Modified: head/sys/dev/acpi_support/acpi_ibm.c
==============================================================================
--- head/sys/dev/acpi_support/acpi_ibm.c        Tue Oct 28 11:20:30 2014        
(r273772)
+++ head/sys/dev/acpi_support/acpi_ibm.c        Tue Oct 28 12:00:39 2014        
(r273773)
@@ -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: head/sys/dev/acpica/acpi.c
==============================================================================
--- head/sys/dev/acpica/acpi.c  Tue Oct 28 11:20:30 2014        (r273772)
+++ head/sys/dev/acpica/acpi.c  Tue Oct 28 12:00:39 2014        (r273773)
@@ -3783,6 +3783,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);
@@ -3806,16 +3807,16 @@ 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;
-               kern_setenv((char *)oidp->oid_arg1, (char *)req->newptr);
-               acpi_set_debugging(NULL);
+       *dbg = 0;
+       kern_setenv((char *)oidp->oid_arg1, temp);
+       acpi_set_debugging(NULL);
     }
     ACPI_SERIAL_END(acpi);
 

Modified: head/sys/dev/asmc/asmc.c
==============================================================================
--- head/sys/dev/asmc/asmc.c    Tue Oct 28 11:20:30 2014        (r273772)
+++ head/sys/dev/asmc/asmc.c    Tue Oct 28 12:00:39 2014        (r273773)
@@ -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: head/sys/kern/kern_ffclock.c
==============================================================================
--- head/sys/kern/kern_ffclock.c        Tue Oct 28 11:20:30 2014        
(r273772)
+++ head/sys/kern/kern_ffclock.c        Tue Oct 28 12:00:39 2014        
(r273773)
@@ -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;
 
-       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;
-                       }
+       /* Return the name of the current active sysclock. */
+       strlcpy(newclock, sysclocks[sysclock_active], sizeof(newclock));
+       error = sysctl_handle_string(oidp, newclock, sizeof(newclock), req);
+
+       /* 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);
 }
 

Modified: head/sys/net/bpf.c
==============================================================================
--- head/sys/net/bpf.c  Tue Oct 28 11:20:30 2014        (r273772)
+++ head/sys/net/bpf.c  Tue Oct 28 12:00:39 2014        (r273773)
@@ -2807,7 +2807,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;
@@ -2827,11 +2828,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: head/sys/netinet/cc/cc.c
==============================================================================
--- head/sys/netinet/cc/cc.c    Tue Oct 28 11:20:30 2014        (r273772)
+++ head/sys/netinet/cc/cc.c    Tue Oct 28 12:00:39 2014        (r273773)
@@ -92,36 +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) {
-                       /* NOTE: "newptr" is not zero terminated */
-                       if (req->newlen != strnlen(funcs->name,
-                           TCP_CA_NAME_MAX - 1))
-                               continue;
-                       if (bcmp(req->newptr, funcs->name, req->newlen))
-                               continue;
-                       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: head/sys/netinet/sctp_sysctl.c
==============================================================================
--- head/sys/netinet/sctp_sysctl.c      Tue Oct 28 11:20:30 2014        
(r273772)
+++ head/sys/netinet/sctp_sysctl.c      Tue Oct 28 12:00:39 2014        
(r273773)
@@ -636,17 +636,26 @@ sctp_sysctl_handle_stats(SYSCTL_HANDLER_
        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_
                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);

Modified: head/sys/netinet/siftr.c
==============================================================================
--- head/sys/netinet/siftr.c    Tue Oct 28 11:20:30 2014        (r273772)
+++ head/sys/netinet/siftr.c    Tue Oct 28 12:00:39 2014        (r273773)
@@ -268,6 +268,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;
@@ -299,7 +300,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,
@@ -1145,38 +1146,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)
 {
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to