Project

General

Profile

Actions
Copy link

Bug #6161

open

spa_load_l2cache() crashes trying to perform zero-sized allocation

Added by Alexander Kolbasov almost 6 years ago. Updated almost 6 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
zfs - Zettabyte File System
Start date:
2015-08-24
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

The illumos-based system crashed with

attempted to kmem_alloc() size of 0
ffffff007aee8a00 vpanic()
ffffff007aee8a40 0xfffffffffbdb1bfe()
ffffff007aee8ad0 spa_load_l2cache+0x92(ffffff17c21c3000)
ffffff007aee8b60 spa_vdev_remove+0x25d(ffffff17c21c3000, 6e9a6f1dbde5bc2d, 0)
ffffff007aee8bb0 zfs_ioc_vdev_remove+0x39(ffffff18b7e11000)
ffffff007aee8c60 zfsdev_ioctl+0x4ff(a500000000, 5a0c, febcca68, 100003, ffffff1229561108, ffffff007aee8e48)
ffffff007aee8ca0 cdev_ioctl+0x39(a500000000, 5a0c, febcca68, 100003, ffffff1229561108, ffffff007aee8e48)
ffffff007aee8cf0 spec_ioctl+0x60(ffffff1225c2df00, 5a0c, febcca68, 100003, ffffff1229561108, ffffff007aee8e48, 0)
ffffff007aee8d80 fop_ioctl+0x55(ffffff1225c2df00, 5a0c, febcca68, 100003, ffffff1229561108, ffffff007aee8e48, 0)
ffffff007aee8ea0 ioctl+0x9b(9, 5a0c, febcca68)
ffffff007aee8f00 _sys_sysenter_post_swapgs+0x241()
Actions
Copy link
 #1

Updated by Alexander Kolbasov almost 6 years ago

The problem occurs here:

static void
spa_load_l2cache(spa_t *spa)
{
    nvlist_t **l2cache;
    uint_t nl2cache;
    int i, j, oldnvdevs;
    uint64_t guid;
    vdev_t *vd, **oldvdevs, **newvdevs;
    spa_aux_vdev_t *sav = &spa->spa_l2cache;

    ASSERT(spa_config_held(spa, SCL_ALL, RW_WRITER) == SCL_ALL);

    if (sav->sav_config != NULL) {
        VERIFY(nvlist_lookup_nvlist_array(sav->sav_config,
            ZPOOL_CONFIG_L2CACHE, &l2cache, &nl2cache) == 0);
        newvdevs = kmem_alloc(nl2cache * sizeof (void *), KM_SLEEP); // <-- Here
    } else 
             ,,,
    }

The l2cache config lists an empty array:

0xffffff171be2f730::nvlist -v

name='l2cache' type=nvpair list_array items=0

So the length is zero.

Actions
Copy link
 #2

Updated by Alexander Kolbasov almost 6 years ago

See https://smartos.org/bugview/OS-4539 for a discussion about similar issue.

Actions
Copy link
 #3

Updated by Alexander Kolbasov almost 6 years ago

Suggested fix:

diff --git a/usr/src/uts/common/fs/zfs/spa.c b/usr/src/uts/common/fs/zfs/spa.c
index 38b1610..b637ad8 100644
--- a/usr/src/uts/common/fs/zfs/spa.c
+++ b/usr/src/uts/common/fs/zfs/spa.c
@@ -1468,10 +1468,11 @@ static void
 spa_load_l2cache(spa_t *spa)
 {
        nvlist_t **l2cache;
-       uint_t nl2cache;
+       uint_t nl2cache = 0;
        int i, j, oldnvdevs;
        uint64_t guid;
-       vdev_t *vd, **oldvdevs, **newvdevs;
+       vdev_t *vd, **oldvdevs;
+       vdev_t **newvdevs = NULL;
        spa_aux_vdev_t *sav = &spa->spa_l2cache;

        ASSERT(spa_config_held(spa, SCL_ALL, RW_WRITER) == SCL_ALL);
@@ -1479,10 +1480,10 @@ spa_load_l2cache(spa_t *spa)
        if (sav->sav_config != NULL) {
                VERIFY(nvlist_lookup_nvlist_array(sav->sav_config,
                    ZPOOL_CONFIG_L2CACHE, &l2cache, &nl2cache) == 0);
-               newvdevs = kmem_alloc(nl2cache * sizeof (void *), KM_SLEEP);
-       } else {
-               nl2cache = 0;
-               newvdevs = NULL;
+               if (nl2cache > 0) {
+                       newvdevs = kmem_alloc(nl2cache * sizeof (void *),
+                                               KM_SLEEP);
+               }
        }

        oldvdevs = sav->sav_vdevs;
Actions
Copy link
 #4

Updated by Alexander Kolbasov almost 6 years ago

The fix wasn't complete, we also need

diff --git a/usr/src/uts/common/fs/zfs/spa.c b/usr/src/uts/common/fs/zfs/spa.c
index b637ad8..2dfccc9 100644
--- a/usr/src/uts/common/fs/zfs/spa.c
+++ b/usr/src/uts/common/fs/zfs/spa.c
@@ -1582,12 +1582,14 @@ spa_load_l2cache(spa_t *spa)
        VERIFY(nvlist_remove(sav->sav_config, ZPOOL_CONFIG_L2CACHE,
            DATA_TYPE_NVLIST_ARRAY) == 0);

-       l2cache = kmem_alloc(sav->sav_count * sizeof (void *), KM_SLEEP);
-       for (i = 0; i < sav->sav_count; i++)
-               l2cache[i] = vdev_config_generate(spa,
-                   sav->sav_vdevs[i], B_TRUE, VDEV_CONFIG_L2CACHE);
-       VERIFY(nvlist_add_nvlist_array(sav->sav_config,
-           ZPOOL_CONFIG_L2CACHE, l2cache, sav->sav_count) == 0);
+       if (sav->sav_count != 0) {
+               l2cache = kmem_alloc(sav->sav_count * sizeof (void *), KM_SLEEP);
+               for (i = 0; i < sav->sav_count; i++)
+                       l2cache[i] = vdev_config_generate(spa,
+                           sav->sav_vdevs[i], B_TRUE, VDEV_CONFIG_L2CACHE);
+               VERIFY(nvlist_add_nvlist_array(sav->sav_config,
+                   ZPOOL_CONFIG_L2CACHE, l2cache, sav->sav_count) == 0);
+       }
 out:
        for (i = 0; i < sav->sav_count; i++)
                nvlist_free(l2cache[i]);
Actions
Copy link

Also available in: Atom PDF