Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/raidframe raidframe: reject invalid values for numCo...



details:   https://anonhg.NetBSD.org/src/rev/eaa558429a72
branches:  trunk
changeset: 368877:eaa558429a72
user:      mrg <mrg%NetBSD.org@localhost>
date:      Wed Aug 10 01:16:38 2022 +0000

description:
raidframe: reject invalid values for numCol and numSpares

numCol and numSpares are "int" so they can be "-1" internally,
which means negative values need to be rejected, as well as
values higher than RF_MAXCOL/RF_MAXSPARES.

explicitly nul-terminate all strings coming from userland.

some minor CSE that avoids signed arith.

this fixes issues in the RAIDFRAME_ADD_HOT_SPARE,
RAIDFRAME_CONFIGURE, RAIDFRAME_DELETE_COMPONENT,
RAIDFRAME_INCORPORATE_HOT_SPARE, and RAIDFRAME_REBUILD_IN_PLACE
ioctl commands.

Reported-by: syzbot+b584943ad1f8ab9d4fe0%syzkaller.appspotmail.com@localhost

https://syzkaller.appspot.com/bug?id=61e07e418261f8eec8a37a9226725fe31820edd0
https://syzkaller.appspot.com/bug?id=ca0c997b40de81c0f0b44790217731f142003149
https://syzkaller.appspot.com/bug?id=6fc452d228453494655a85264591dd9054cc0b08
https://syzkaller.appspot.com/bug?id=873f0271682713a27adc9a49dd7109c70b35fda3


XXX: pullup-8, pullup-9.

ok oster@ riastradh@

diffstat:

 sys/dev/raidframe/rf_disks.c       |  15 +++++-----
 sys/dev/raidframe/rf_driver.c      |   9 +++++-
 sys/dev/raidframe/rf_netbsdkintf.c |  51 ++++++++++++++++++++++++++++++-------
 3 files changed, 55 insertions(+), 20 deletions(-)

diffs (192 lines):

diff -r d29312171bc7 -r eaa558429a72 sys/dev/raidframe/rf_disks.c
--- a/sys/dev/raidframe/rf_disks.c      Wed Aug 10 00:28:00 2022 +0000
+++ b/sys/dev/raidframe/rf_disks.c      Wed Aug 10 01:16:38 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: rf_disks.c,v 1.92 2019/12/08 12:14:40 mlelstv Exp $    */
+/*     $NetBSD: rf_disks.c,v 1.93 2022/08/10 01:16:38 mrg Exp $        */
 /*-
  * Copyright (c) 1999 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -60,7 +60,7 @@
  ***************************************************************/
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: rf_disks.c,v 1.92 2019/12/08 12:14:40 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: rf_disks.c,v 1.93 2022/08/10 01:16:38 mrg Exp $");
 
 #include <dev/raidframe/raidframevar.h>
 
@@ -318,11 +318,12 @@
 rf_AllocDiskStructures(RF_Raid_t *raidPtr, RF_Config_t *cfgPtr)
 {
        int ret;
+       size_t entries = raidPtr->numCol + RF_MAXSPARE;
 
        /* We allocate RF_MAXSPARE on the first row so that we
           have room to do hot-swapping of spares */
-       raidPtr->Disks = RF_MallocAndAdd((raidPtr->numCol + RF_MAXSPARE) *
-           sizeof(*raidPtr->Disks), raidPtr->cleanupList);
+       raidPtr->Disks = RF_MallocAndAdd(
+           entries * sizeof(*raidPtr->Disks), raidPtr->cleanupList);
        if (raidPtr->Disks == NULL) {
                ret = ENOMEM;
                goto fail;
@@ -330,9 +331,7 @@
 
        /* get space for device specific stuff.. */
        raidPtr->raid_cinfo = RF_MallocAndAdd(
-           (raidPtr->numCol + RF_MAXSPARE) * sizeof(*raidPtr->raid_cinfo),
-           raidPtr->cleanupList);
-
+           entries * sizeof(*raidPtr->raid_cinfo), raidPtr->cleanupList);
        if (raidPtr->raid_cinfo == NULL) {
                ret = ENOMEM;
                goto fail;
@@ -607,7 +606,7 @@
        error = vn_bdev_openpath(pb, &vp, curlwp);
        pathbuf_destroy(pb);
        if (error) {
-               printf("open device: %s failed!\n", diskPtr->devname);
+               printf("open device: '%s' failed: %d\n", diskPtr->devname, error);
                if (error == ENXIO) {
                        /* the component isn't there... must be dead :-( */
                        diskPtr->status = rf_ds_failed;
diff -r d29312171bc7 -r eaa558429a72 sys/dev/raidframe/rf_driver.c
--- a/sys/dev/raidframe/rf_driver.c     Wed Aug 10 00:28:00 2022 +0000
+++ b/sys/dev/raidframe/rf_driver.c     Wed Aug 10 01:16:38 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: rf_driver.c,v 1.139 2021/07/23 02:35:14 oster Exp $    */
+/*     $NetBSD: rf_driver.c,v 1.140 2022/08/10 01:16:38 mrg Exp $      */
 /*-
  * Copyright (c) 1999 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -66,7 +66,7 @@
 
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: rf_driver.c,v 1.139 2021/07/23 02:35:14 oster Exp $");
+__KERNEL_RCSID(0, "$NetBSD: rf_driver.c,v 1.140 2022/08/10 01:16:38 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_raid_diagnostic.h"
@@ -341,6 +341,11 @@
                          (void (*) (void *)) rf_FreeAllocList,
                          raidPtr->cleanupList);
 
+       KASSERT(cfgPtr->numCol < RF_MAXCOL);
+       KASSERT(cfgPtr->numCol >= 0);
+       KASSERT(cfgPtr->numSpare < RF_MAXSPARE);
+       KASSERT(cfgPtr->numSpare >= 0);
+
        raidPtr->numCol = cfgPtr->numCol;
        raidPtr->numSpare = cfgPtr->numSpare;
 
diff -r d29312171bc7 -r eaa558429a72 sys/dev/raidframe/rf_netbsdkintf.c
--- a/sys/dev/raidframe/rf_netbsdkintf.c        Wed Aug 10 00:28:00 2022 +0000
+++ b/sys/dev/raidframe/rf_netbsdkintf.c        Wed Aug 10 01:16:38 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: rf_netbsdkintf.c,v 1.407 2022/04/16 16:40:54 andvar Exp $      */
+/*     $NetBSD: rf_netbsdkintf.c,v 1.408 2022/08/10 01:16:38 mrg Exp $ */
 
 /*-
  * Copyright (c) 1996, 1997, 1998, 2008-2011 The NetBSD Foundation, Inc.
@@ -101,7 +101,7 @@
  ***********************************************************/
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: rf_netbsdkintf.c,v 1.407 2022/04/16 16:40:54 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: rf_netbsdkintf.c,v 1.408 2022/08/10 01:16:38 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_raid_autoconfig.h"
@@ -1240,7 +1240,7 @@
 int
 rf_construct(struct raid_softc *rs, RF_Config_t *k_cfg)
 {
-       int retcode;
+       int retcode, i;
        RF_Raid_t *raidPtr = &rs->sc_r;
 
        rs->sc_flags &= ~RAIDF_SHUTDOWN;
@@ -1251,6 +1251,29 @@
        /* should do some kind of sanity check on the configuration.
         * Store the sum of all the bytes in the last byte? */
 
+       /* Force nul-termination on all strings. */
+#define ZERO_FINAL(s)  do { s[sizeof(s) - 1] = '\0'; } while (0)
+       for (i = 0; i < RF_MAXCOL; i++) {
+               ZERO_FINAL(k_cfg->devnames[0][i]);
+       }
+       for (i = 0; i < RF_MAXSPARE; i++) {
+               ZERO_FINAL(k_cfg->spare_names[i]);
+       }
+       for (i = 0; i < RF_MAXDBGV; i++) {
+               ZERO_FINAL(k_cfg->debugVars[i]);
+       }
+#undef ZERO_FINAL
+
+       /* Check some basic limits. */
+       if (k_cfg->numCol >= RF_MAXCOL || k_cfg->numCol < 0) {
+               retcode = EINVAL;
+               goto out;
+       }
+       if (k_cfg->numSpare >= RF_MAXSPARE || k_cfg->numSpare < 0) {
+               retcode = EINVAL;
+               goto out;
+       }
+
        /* configure the system */
 
        /*
@@ -1451,6 +1474,18 @@
        return 0;
 }
 
+/*
+ * Copy a RF_SingleComponent_t from 'data', ensuring nul-termination
+ * on the component_name[] array.
+ */
+static void
+rf_copy_single_component(RF_SingleComponent_t *component, void *data)
+{
+
+       memcpy(component, data, sizeof *component);
+       component->component_name[sizeof(component->component_name) - 1] = '\0';
+}
+
 static int
 raidioctl(dev_t dev, u_long cmd, void *data, int flag, struct lwp *l)
 {
@@ -1466,7 +1501,6 @@
        int retcode = 0;
        int column;
        RF_ComponentLabel_t *clabel;
-       RF_SingleComponent_t *sparePtr,*componentPtr;
        int d;
 
        if ((rs = raidget(unit, false)) == NULL)
@@ -1555,21 +1589,18 @@
                    rf_RewriteParityThread, raidPtr,"raid_parity");
 
        case RAIDFRAME_ADD_HOT_SPARE:
-               sparePtr = (RF_SingleComponent_t *) data;
-               memcpy(&component, sparePtr, sizeof(RF_SingleComponent_t));
+               rf_copy_single_component(&component, data);
                return rf_add_hot_spare(raidPtr, &component);
 
        case RAIDFRAME_REMOVE_HOT_SPARE:
                return retcode;
 
        case RAIDFRAME_DELETE_COMPONENT:
-               componentPtr = (RF_SingleComponent_t *)data;
-               memcpy(&component, componentPtr, sizeof(RF_SingleComponent_t));
+               rf_copy_single_component(&component, data);
                return rf_delete_component(raidPtr, &component);
 
        case RAIDFRAME_INCORPORATE_HOT_SPARE:
-               componentPtr = (RF_SingleComponent_t *)data;
-               memcpy(&component, componentPtr, sizeof(RF_SingleComponent_t));
+               rf_copy_single_component(&component, data);
                return rf_incorporate_hot_spare(raidPtr, &component);
 
        case RAIDFRAME_REBUILD_IN_PLACE:



Home | Main Index | Thread Index | Old Index