From: Matthew Ahrens Date: Wed, 13 Jan 2016 18:45:08 +0000 (-0800) Subject: OpenZFS 7263 - deeply nested nvlist can overflow stack X-Git-Tag: zfs-0.7.0-rc1~41 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=169ab07cc8b7417c4a93b4496ef4a11e049b5cdc;p=zfs OpenZFS 7263 - deeply nested nvlist can overflow stack nvlist_pack() and nvlist_unpack are implemented recursively, which can cause the stack to overflow with a deeply nested nvlist; i.e. an nvlist which contains an nvlist, which contains an nvlist, which... Unprivileged users can pass an nvlist to the kernel via certain ioctls on /dev/zfs, which the kernel will unpack without additional permission checking or validation. Therefore, an unprivileged user can cause the kernel's stack to overflow and panic. Ideally, these functions would be implemented non-recursively. As a quick fix, this patch limits the depth of the recursion and returns an error when attempting to pack and unpack a deeply-nested nvlist. Signed-off-by: Adam Leventhal Signed-off-by: George Wilson Signed-off-by: Brian Behlendorf Ported-by: Prakash Surya OpenZFS-issue: https://www.illumos.org/issues/7263 OpenZFS-commit: https://github.com/openzfs/openzfs/commit/0511d6d - --- diff --git a/module/nvpair/nvpair.c b/module/nvpair/nvpair.c index bfd21314e..071923ea7 100644 --- a/module/nvpair/nvpair.c +++ b/module/nvpair/nvpair.c @@ -21,6 +21,7 @@ /* * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2016 by Delphix. All rights reserved. */ #include @@ -138,6 +139,11 @@ static int nvlist_add_common(nvlist_t *nvl, const char *name, data_type_t type, #define NVPAIR2I_NVP(nvp) \ ((i_nvp_t *)((size_t)(nvp) - offsetof(i_nvp_t, nvi_nvp))) +#ifdef _KERNEL +int nvpair_max_recursion = 20; +#else +int nvpair_max_recursion = 100; +#endif int nv_alloc_init(nv_alloc_t *nva, const nv_alloc_ops_t *nvo, /* args */ ...) @@ -2017,6 +2023,7 @@ typedef struct { const nvs_ops_t *nvs_ops; void *nvs_private; nvpriv_t *nvs_priv; + int nvs_recursion; } nvstream_t; /* @@ -2168,9 +2175,16 @@ static int nvs_embedded(nvstream_t *nvs, nvlist_t *embedded) { switch (nvs->nvs_op) { - case NVS_OP_ENCODE: - return (nvs_operation(nvs, embedded, NULL)); + case NVS_OP_ENCODE: { + int err; + if (nvs->nvs_recursion >= nvpair_max_recursion) + return (EINVAL); + nvs->nvs_recursion++; + err = nvs_operation(nvs, embedded, NULL); + nvs->nvs_recursion--; + return (err); + } case NVS_OP_DECODE: { nvpriv_t *priv; int err; @@ -2183,8 +2197,12 @@ nvs_embedded(nvstream_t *nvs, nvlist_t *embedded) nvlist_init(embedded, embedded->nvl_nvflag, priv); + if (nvs->nvs_recursion >= nvpair_max_recursion) + return (EINVAL); + nvs->nvs_recursion++; if ((err = nvs_operation(nvs, embedded, NULL)) != 0) nvlist_free(embedded); + nvs->nvs_recursion--; return (err); } default: @@ -2272,6 +2290,7 @@ nvlist_common(nvlist_t *nvl, char *buf, size_t *buflen, int encoding, return (EINVAL); nvs.nvs_op = nvs_op; + nvs.nvs_recursion = 0; /* * For NVS_OP_ENCODE and NVS_OP_DECODE make sure an nvlist and