Skip to content

Commit

Permalink
OpenZFS 7263 - deeply nested nvlist can overflow stack
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: George Wilson <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Ported-by: Prakash Surya <[email protected]>

OpenZFS-issue: https://www.illumos.org/issues/7263
OpenZFS-commit: openzfs/openzfs@0511d6d

-
  • Loading branch information
ahrens authored and nedbass committed Sep 9, 2016
1 parent 58000c3 commit 1421562
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions module/nvpair/nvpair.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 <sys/stropts.h>
Expand Down Expand Up @@ -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 */ ...)
Expand Down Expand Up @@ -2017,6 +2023,7 @@ typedef struct {
const nvs_ops_t *nvs_ops;
void *nvs_private;
nvpriv_t *nvs_priv;
int nvs_recursion;
} nvstream_t;

/*
Expand Down Expand Up @@ -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;
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1421562

Please sign in to comment.