--- sys/contrib/openzfs/module/nvpair/nvpair.c.orig +++ sys/contrib/openzfs/module/nvpair/nvpair.c @@ -134,7 +134,8 @@ #define NVP_SIZE_CALC(name_len, data_len) \ (NV_ALIGN((sizeof (nvpair_t)) + name_len) + NV_ALIGN(data_len)) -static int i_get_value_size(data_type_t type, const void *data, uint_t nelem); +static int i_get_value_size(data_type_t type, const void *data, uint_t nelem, + size_t max_size); static int nvlist_add_common(nvlist_t *nvl, const char *name, data_type_t type, uint_t nelem, const void *data); @@ -809,8 +810,10 @@ * verify nvp_type, nvp_value_elem, and also possibly * verify string values and get the value size. */ - size2 = i_get_value_size(type, NVP_VALUE(nvp), NVP_NELEM(nvp)); size1 = nvp->nvp_size - NVP_VALOFF(nvp); + size2 = i_get_value_size(type, NVP_VALUE(nvp), NVP_NELEM(nvp), + size1); + if (size2 < 0 || size1 != NV_ALIGN(size2)) return (EFAULT); @@ -1001,12 +1004,21 @@ * DATA_TYPE_STRING and * DATA_TYPE_STRING_ARRAY * Is data == NULL then the size of the string(s) is excluded. + * + * If 'max_size' is non-zero, then don't look beyond 'max_size' number of + * bytes when calculating a value size. Note that 'max_size' should include + * the NULL terminator byte when calculating string size. If 'max_size' is 0, + * it is ignored. */ static int -i_get_value_size(data_type_t type, const void *data, uint_t nelem) +i_get_value_size(data_type_t type, const void *data, uint_t nelem, + size_t max_size) { uint64_t value_sz; + if (max_size == 0) + max_size = INT32_MAX; + if (i_validate_type_nelem(type, nelem) != 0) return (-1); @@ -1051,10 +1063,15 @@ break; #endif case DATA_TYPE_STRING: - if (data == NULL) + if (data == NULL) { value_sz = 0; - else - value_sz = strlen(data) + 1; + } else { + value_sz = strnlen(data, max_size); + if (value_sz >= max_size) { + return (-1); /* string not terminated */ + } + value_sz += 1; + } break; case DATA_TYPE_BOOLEAN_ARRAY: value_sz = (uint64_t)nelem * sizeof (boolean_t); @@ -1088,16 +1105,23 @@ break; case DATA_TYPE_STRING_ARRAY: value_sz = (uint64_t)nelem * sizeof (uint64_t); - if (data != NULL) { char *const *strs = data; uint_t i; + size_t newsize; /* no alignment requirement for strings */ for (i = 0; i < nelem; i++) { if (strs[i] == NULL) return (-1); - value_sz += strlen(strs[i]) + 1; + + newsize = strnlen(strs[i], max_size); + + if (newsize == max_size) + return (-1); /* not terminated */ + + value_sz += newsize + 1; /* +1 for NULL */ + max_size -= newsize + 1; } } break; @@ -1162,7 +1186,7 @@ * In case of data types DATA_TYPE_STRING and DATA_TYPE_STRING_ARRAY * is the size of the string(s) included. */ - if ((value_sz = i_get_value_size(type, data, nelem)) < 0) + if ((value_sz = i_get_value_size(type, data, nelem, 0)) < 0) return (EINVAL); if (i_validate_nvpair_value(type, nelem, data) != 0) @@ -1587,7 +1611,7 @@ #endif if (data == NULL) return (EINVAL); - if ((value_sz = i_get_value_size(type, NULL, 1)) < 0) + if ((value_sz = i_get_value_size(type, NULL, 1, 0)) < 0) return (EINVAL); memcpy(data, NVP_VALUE(nvp), (size_t)value_sz); if (nelem != NULL) @@ -3017,7 +3041,8 @@ * In case of data types DATA_TYPE_STRING and DATA_TYPE_STRING_ARRAY * is the size of the string(s) excluded. */ - if ((value_sz = i_get_value_size(type, NULL, NVP_NELEM(nvp))) < 0) + if ((value_sz = i_get_value_size(type, NULL, NVP_NELEM(nvp), + NVP_SIZE(nvp))) < 0) return (EFAULT); if (NVP_SIZE_CALC(nvp->nvp_name_sz, value_sz) > nvp->nvp_size) @@ -3332,7 +3357,7 @@ * In case of data types DATA_TYPE_STRING and DATA_TYPE_STRING_ARRAY * is the size of the string(s) excluded. */ - if ((value_sz = i_get_value_size(type, NULL, nelem)) < 0) + if ((value_sz = i_get_value_size(type, NULL, nelem, NVP_SIZE(nvp)) < 0)) return (EFAULT); /* if there is no data to extract then return */ --- sys/contrib/openzfs/module/zfs/dmu_recv.c.orig +++ sys/contrib/openzfs/module/zfs/dmu_recv.c @@ -2834,16 +2834,20 @@ { struct drr_object *drro = &drc->drc_rrd->header.drr_u.drr_object; - uint32_t size = DRR_OBJECT_PAYLOAD_SIZE(drro); + uint32_t size; void *buf = NULL; dmu_object_info_t doi; + size = DRR_OBJECT_PAYLOAD_SIZE(drro); + if (size > SPA_MAXBLOCKSIZE) + return (SET_ERROR(ERANGE)); + if (size != 0) - buf = kmem_zalloc(size, KM_SLEEP); + buf = vmem_zalloc(size, KM_SLEEP); err = receive_read_payload_and_next_header(drc, size, buf); if (err != 0) { - kmem_free(buf, size); + vmem_free(buf, size); return (err); } err = dmu_object_info(drc->drc_os, drro->drr_object, &doi); @@ -2867,7 +2871,11 @@ case DRR_WRITE: { struct drr_write *drrw = &drc->drc_rrd->header.drr_u.drr_write; - int size = DRR_WRITE_PAYLOAD_SIZE(drrw); + uint64_t size = DRR_WRITE_PAYLOAD_SIZE(drrw); + + if (size > SPA_MAXBLOCKSIZE) + return (SET_ERROR(ERANGE)); + abd_t *abd = abd_alloc_linear(size, B_FALSE); err = receive_read_payload_and_next_header(drc, size, abd_to_buf(abd)); @@ -2884,12 +2892,18 @@ { struct drr_write_embedded *drrwe = &drc->drc_rrd->header.drr_u.drr_write_embedded; - uint32_t size = P2ROUNDUP(drrwe->drr_psize, 8); - void *buf = kmem_zalloc(size, KM_SLEEP); + uint32_t size; + void *buf; + + size = P2ROUNDUP(drrwe->drr_psize, 8); + if (size > SPA_MAXBLOCKSIZE) + return (SET_ERROR(ERANGE)); + + buf = vmem_zalloc(size, KM_SLEEP); err = receive_read_payload_and_next_header(drc, size, buf); if (err != 0) { - kmem_free(buf, size); + vmem_free(buf, size); return (err); } @@ -2918,7 +2932,11 @@ case DRR_SPILL: { struct drr_spill *drrs = &drc->drc_rrd->header.drr_u.drr_spill; - int size = DRR_SPILL_PAYLOAD_SIZE(drrs); + uint64_t size = DRR_SPILL_PAYLOAD_SIZE(drrs); + + if (size > SPA_MAXBLOCKSIZE) + return (SET_ERROR(ERANGE)); + abd_t *abd = abd_alloc_linear(size, B_FALSE); err = receive_read_payload_and_next_header(drc, size, abd_to_buf(abd)); @@ -3069,7 +3087,7 @@ abd_free(rrd->abd); rrd->abd = NULL; } else if (rrd->payload != NULL) { - kmem_free(rrd->payload, rrd->payload_size); + vmem_free(rrd->payload, rrd->payload_size); rrd->payload = NULL; } return (0); @@ -3083,7 +3101,7 @@ rrd->abd = NULL; rrd->payload = NULL; } else if (rrd->payload != NULL) { - kmem_free(rrd->payload, rrd->payload_size); + vmem_free(rrd->payload, rrd->payload_size); rrd->payload = NULL; } @@ -3096,7 +3114,7 @@ { struct drr_object *drro = &rrd->header.drr_u.drr_object; err = receive_object(rwa, drro, rrd->payload); - kmem_free(rrd->payload, rrd->payload_size); + vmem_free(rrd->payload, rrd->payload_size); rrd->payload = NULL; break; } @@ -3134,7 +3152,7 @@ struct drr_write_embedded *drrwe = &rrd->header.drr_u.drr_write_embedded; err = receive_write_embedded(rwa, drrwe, rrd->payload); - kmem_free(rrd->payload, rrd->payload_size); + vmem_free(rrd->payload, rrd->payload_size); rrd->payload = NULL; break; } @@ -3203,7 +3221,7 @@ rrd->abd = NULL; rrd->payload = NULL; } else if (rrd->payload != NULL) { - kmem_free(rrd->payload, rrd->payload_size); + vmem_free(rrd->payload, rrd->payload_size); rrd->payload = NULL; } /* --- sys/contrib/openzfs/module/zfs/vdev_label.c.orig +++ sys/contrib/openzfs/module/zfs/vdev_label.c @@ -1362,6 +1362,7 @@ VB_NVLIST); break; } + vbe->vbe_bootenv[sizeof (vbe->vbe_bootenv) - 1] = '\0'; fnvlist_add_string(bootenv, FREEBSD_BOOTONCE, buf); } --- sys/contrib/openzfs/module/zfs/zfs_ioctl.c.orig +++ sys/contrib/openzfs/module/zfs/zfs_ioctl.c @@ -913,6 +913,23 @@ ZFS_DELEG_PERM_CREATE, cr)); } +/* + * Policy for dataset set property operations. Individual properties checked by + * zfs_check_settable(), additionally require zfs_secpolicy_recv() when setting + * properties as part of a receive. + */ +static int +zfs_secpolicy_setprops(zfs_cmd_t *zc, nvlist_t *innvl, cred_t *cr) +{ + boolean_t received = zc->zc_cookie; + int error; + + if (received && (error = zfs_secpolicy_recv(zc, innvl, cr))) + return (error); + + return (zfs_secpolicy_read(zc, innvl, cr)); +} + int zfs_secpolicy_snapshot_perms(const char *name, cred_t *cr) { @@ -3642,7 +3659,6 @@ zfs_ioc_log_history(const char *unused, nvlist_t *innvl, nvlist_t *outnvl) { (void) unused, (void) outnvl; - const char *message; char *poolname; spa_t *spa; int error; @@ -3663,7 +3679,7 @@ if (error != 0) return (error); - message = fnvlist_lookup_string(innvl, "message"); + const char *message = fnvlist_lookup_string(innvl, "message"); if (spa_version(spa) < SPA_VERSION_ZPOOL_HISTORY) { spa_close(spa, FTAG); @@ -5994,21 +6010,27 @@ * outputs: * zc_nvlist_dst[_size] data buffer (array of zfs_useracct_t) * zc_cookie zap cursor + * + * The zc_nvlist_dst output array is limited to 1000 entries. */ static int zfs_ioc_userspace_many(zfs_cmd_t *zc) { + const size_t batch_limit = 1000 * sizeof (zfs_useracct_t); + uint64_t bufsize = MIN(zc->zc_nvlist_dst_size, batch_limit); zfsvfs_t *zfsvfs; - int bufsize = zc->zc_nvlist_dst_size; - if (bufsize <= 0) + if (bufsize < sizeof (zfs_useracct_t)) { + zc->zc_nvlist_dst_size = sizeof (zfs_useracct_t); return (SET_ERROR(ENOMEM)); + } int error = zfsvfs_hold(zc->zc_name, FTAG, &zfsvfs, B_FALSE); if (error != 0) return (error); void *buf = vmem_alloc(bufsize, KM_SLEEP); + zc->zc_nvlist_dst_size = bufsize; error = zfs_userspace_many(zfsvfs, zc->zc_objset_type, &zc->zc_cookie, buf, &zc->zc_nvlist_dst_size); @@ -6499,7 +6521,7 @@ dsl_pool_t *dp; dsl_dataset_t *new, *old; const char *firstsnap; - uint64_t used, comp, uncomp; + uint64_t used = 0, comp = 0, uncomp = 0; firstsnap = fnvlist_lookup_string(innvl, "firstsnap"); @@ -7374,7 +7396,7 @@ zfs_ioc_send, zfs_secpolicy_send); zfs_ioctl_register_dataset_modify(ZFS_IOC_SET_PROP, zfs_ioc_set_prop, - zfs_secpolicy_none); + zfs_secpolicy_setprops); zfs_ioctl_register_dataset_modify(ZFS_IOC_DESTROY, zfs_ioc_destroy, zfs_secpolicy_destroy); zfs_ioctl_register_dataset_modify(ZFS_IOC_RENAME, zfs_ioc_rename, --- sys/contrib/openzfs/module/zfs/zfs_quota.c.orig +++ sys/contrib/openzfs/module/zfs/zfs_quota.c @@ -85,10 +85,14 @@ sa.sa_layout_info = BSWAP_16(sa.sa_layout_info); swap = B_TRUE; } - VERIFY3U(sa.sa_magic, ==, SA_MAGIC); + + if (unlikely(sa.sa_magic != SA_MAGIC)) + return (SET_ERROR(EINVAL)); int hdrsize = sa_hdrsize(&sa); - VERIFY3U(hdrsize, >=, sizeof (sa_hdr_phys_t)); + + if (unlikely(hdrsize < sizeof (sa_hdr_phys_t))) + return (SET_ERROR(EINVAL)); uintptr_t data_after_hdr = (uintptr_t)data + hdrsize; zoi->zfi_user = *((uint64_t *)(data_after_hdr + SA_UID_OFFSET)); --- sys/contrib/openzfs/tests/zfs-tests/cmd/libzfs_input_check.c.orig +++ sys/contrib/openzfs/tests/zfs-tests/cmd/libzfs_input_check.c @@ -84,7 +84,6 @@ ZFS_IOC_DSOBJ_TO_DSNAME, ZFS_IOC_OBJ_TO_PATH, ZFS_IOC_POOL_SET_PROPS, - ZFS_IOC_POOL_GET_PROPS, ZFS_IOC_SET_FSACL, ZFS_IOC_GET_FSACL, ZFS_IOC_SHARE, @@ -124,11 +123,136 @@ lzc_ioctl_test(ioc, name, req, opt, err, wild); \ } while (0) +#define IOC_INPUT_TEST_INJECT(ioc, name, innvl) \ + do { \ + active_test = __func__ + 5; \ + lzc_ioctl_run_impl(ioc, name, innvl, 0, B_TRUE); \ + } while (0) + +/* + * Given a zfs_cmd_t containing an already packed nvlist in zc->zc_nvlist_src, + * and its original innvl, look in innvl for the last string nvpair, or last + * string array nvpair, and remove the string terminator. The idea is to + * corrupt the nvlist string value so that anyone doing a strlen() on it will + * read past the end of the packed nvlist buffer and trigger a crash. + */ +static void +do_bad_string(zfs_cmd_t *zc, nvlist_t *innvl) +{ + nvpair_t *elem = NULL; + nvpair_t *lastseen = NULL; + const char *str = NULL; + const char **arr; + uint_t n; + char *off; + char *packed; + uint64_t size, off_size; + + while ((elem = nvlist_next_nvpair(innvl, elem)) != NULL) { + if ((nvpair_type(elem) == DATA_TYPE_STRING) || + (nvpair_type(elem) == DATA_TYPE_STRING_ARRAY)) + lastseen = elem; + } + + if (lastseen == NULL) + return; /* No strings */ + + /* + * Lookup either the last string, or the last string in the last + * string array in the nvlist. We will use this to corrupt from the + * string to the end of the nvlist buffer. Any attempts to strlen this + * string should run pass the end of the packed buffer. + */ + if (nvpair_value_string(lastseen, &str) != 0) { + if (nvpair_value_string_array(lastseen, &arr, &n) == 0) + str = arr[n-1]; + } + + /* + * We now have the last string. Corrupt everything from the NULL + * terminator byte for the last string to the end of the packed nvlist + * buffer. + */ + packed = (char *)zc->zc_nvlist_src; + size = zc->zc_nvlist_src_size; + + off = memmem(packed, size, str, strlen(str)); + off_size = strlen(str); + + memset(&off[off_size - 1], '!', (packed + size) - + (&off[off_size - 1])); + +} + +/* + * For each byte in the packed nvlist list in zc, corrupt a single byte, then + * try doing the ioctl. This tests how well the kernel handles fuzzed nvlists. + * + * NOTE - make sure you are doing this with a "safe" ioctl! You don't want to + * run this on an ioctl that can potentially corrupt data (like a zpool create). + */ +static void +do_fuzz(int zfs_fd, zfs_ioc_t ioc, zfs_cmd_t *zc) +{ + uint64_t size; + uint64_t i; + unsigned char old = 0; + unsigned char *pos; + zfs_cmd_t orig_zc = *zc; + + pos = (unsigned char *) zc->zc_nvlist_src; + size = zc->zc_nvlist_src_size; + + /* + * Fuzz each byte in the packed nvlist, one byte at a time, and do the + * ioctl. If the kernel doesn't crash, then the test passed. + */ + for (i = 0; i < size; i++) { + /* Restore the previously corrupted byte */ + if (i > 0) + pos[i-1] = old; + + old = pos[i]; + + /* Corrupt the new byte */ + pos[i]++; + + /* + * Do the ioctl and ignore the return code. We just want to + * see if the kernel panics. + */ + lzc_ioctl_fd(zfs_fd, ioc, zc); + + /* + * Restore 'zc' with original fields since the ioctl may + * have modified them. + */ + *zc = orig_zc; + } + /* Restore last byte */ + if (i > 0) + pos[i - 1] = old; + + /* + * Try fuzzing the packed nvlist size field. Test it with one byte + * bigger and one byte smaller than the current value. + */ + zc->zc_nvlist_src_size--; + lzc_ioctl_fd(zfs_fd, ioc, zc); + + zc->zc_nvlist_src_size += 2; + lzc_ioctl_fd(zfs_fd, ioc, zc); + + /* Restore to normal */ + zc->zc_nvlist_src_size -= 1; +} + /* * run a zfs ioctl command, verify expected results and log failures */ static void -lzc_ioctl_run(zfs_ioc_t ioc, const char *name, nvlist_t *innvl, int expected) +lzc_ioctl_run_impl(zfs_ioc_t ioc, const char *name, nvlist_t *innvl, + int expected, boolean_t do_corrupt) { zfs_cmd_t zc = {"\0"}; char *packed = NULL; @@ -159,10 +283,30 @@ zc.zc_nvlist_dst_size = MAX(size * 2, 128 * 1024); zc.zc_nvlist_dst = (uint64_t)(uintptr_t)malloc(zc.zc_nvlist_dst_size); + if (do_corrupt) { + /* + * Try changing bytes in the packed nvlist to see if it will + * panic the kernel when you do the ioctl. + */ + do_fuzz(zfs_fd, ioc, &zc); + + /* + * Corrupt the last string in the packed nvlist so it has no + * NULL terminator. + */ + do_bad_string(&zc, innvl); + + } + if (lzc_ioctl_fd(zfs_fd, ioc, &zc) != 0) error = errno; - if (error != expected) { + /* + * If we're corrupting the nvlist we don't care about the specific + * error code that gets returned, as it could be one of many. We only + * care if it panics the kernel. + */ + if (!do_corrupt && error != expected) { unexpected_failures = B_TRUE; (void) fprintf(stderr, "%s: Unexpected result with %s, " "error %d (expecting %d)\n", @@ -173,6 +317,12 @@ free((void *)(uintptr_t)zc.zc_nvlist_dst); } +static void +lzc_ioctl_run(zfs_ioc_t ioc, const char *name, nvlist_t *innvl, int expected) +{ + return (lzc_ioctl_run_impl(ioc, name, innvl, expected, B_FALSE)); +} + /* * Test each ioc for the following ioctl input errors: * ZFS_ERR_IOC_ARG_UNAVAIL an input argument is not supported by kernel @@ -309,6 +459,7 @@ fnvlist_add_string(required, "message", "input check"); IOC_INPUT_TEST(ZFS_IOC_LOG_HISTORY, pool, required, NULL, 0); + IOC_INPUT_TEST_INJECT(ZFS_IOC_LOG_HISTORY, pool, required); nvlist_free(required); } @@ -790,6 +941,20 @@ nvlist_free(required); } +static void +test_zpool_get(const char *pool) +{ + const char *strs[] = {ZPOOL_DEDUPCACHED_PROP_NAME}; + nvlist_t *optional = fnvlist_alloc(); + + fnvlist_add_string_array(optional, ZPOOL_GET_PROPS_NAMES, strs, 1); + + IOC_INPUT_TEST(ZFS_IOC_POOL_GET_PROPS, pool, NULL, optional, 0); + IOC_INPUT_TEST_INJECT(ZFS_IOC_POOL_GET_PROPS, pool, optional); + + nvlist_free(optional); +} + static void zfs_ioc_input_tests(const char *pool) { @@ -884,6 +1049,7 @@ test_scrub(pool); + test_zpool_get(pool); /* * cleanup */