Date: Sun, 3 Jan 2016 09:08:34 +0000 (UTC) From: Garrett Cooper <ngie@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r293092 - in user/ngie/stable-10-libnv: share/man/man9 sys/contrib/libnv Message-ID: <201601030908.u0398Yoc026138@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ngie Date: Sun Jan 3 09:08:34 2016 New Revision: 293092 URL: https://svnweb.freebsd.org/changeset/base/293092 Log: MFC r286642,r286644,r286645,r286646: r286642 (by oshogbo): Make the nvlist_next(9) function handle NULL pointer variable. This simplifies removing the first element from nvlist. r286644 (by oshogbo): Don't set parent if the unpack operation fail. In some case this could crash the library, because of the NULL pointer references. Discovered thanks to american fuzzy lop. r286645 (by oshogbo): The nvlist_move_nvpair() function can fail in two cases, if: - the nvlist error is set, or - the nvlist case ignore flag is not set and there is attend to add element with duplicated name. In both cases the nvlist_move_nvpair() function free nvpair structure. If library will try to unpack a binary blob which contains duplicated names it will end up with using memory after free. To prevent that, the nvlist_move_nvpair() function interface is changed to report about failure and checks are added to the nvpair_xunpack() function. Discovered thanks to the american fuzzy lop. r286646 (by oshogbo): If any function fail (the ptr variable will be equal to NULL), we shouldn't return buffer. Instead we should free it and return NULL. Modified: user/ngie/stable-10-libnv/share/man/man9/nv.9 user/ngie/stable-10-libnv/sys/contrib/libnv/nv_impl.h user/ngie/stable-10-libnv/sys/contrib/libnv/nvlist.c Directory Properties: user/ngie/stable-10-libnv/ (props changed) Modified: user/ngie/stable-10-libnv/share/man/man9/nv.9 ============================================================================== --- user/ngie/stable-10-libnv/share/man/man9/nv.9 Sun Jan 3 08:48:23 2016 (r293091) +++ user/ngie/stable-10-libnv/share/man/man9/nv.9 Sun Jan 3 09:08:34 2016 (r293092) @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd July 4, 2015 +.Dd Aug 11, 2015 .Dt NV 9 .Os .Sh NAME @@ -410,6 +410,16 @@ The argument can be NULL. Elements may not be removed from the nvlist while traversing it. The nvlist must not be in error state. +Note that +.Fn nvlist_next +will handle +.Va cookiep +being set to +.Dv NULL . +In this case first element is returned or +.Dv NULL +if nvlist is empty. +This behavior simplifies removing the first element from the list. .Pp The .Fn nvlist_exists Modified: user/ngie/stable-10-libnv/sys/contrib/libnv/nv_impl.h ============================================================================== --- user/ngie/stable-10-libnv/sys/contrib/libnv/nv_impl.h Sun Jan 3 08:48:23 2016 (r293091) +++ user/ngie/stable-10-libnv/sys/contrib/libnv/nv_impl.h Sun Jan 3 09:08:34 2016 (r293092) @@ -93,7 +93,7 @@ nvpair_t *nvlist_prev_nvpair(const nvlis void nvlist_add_nvpair(nvlist_t *nvl, const nvpair_t *nvp); -void nvlist_move_nvpair(nvlist_t *nvl, nvpair_t *nvp); +bool nvlist_move_nvpair(nvlist_t *nvl, nvpair_t *nvp); void nvlist_set_parent(nvlist_t *nvl, nvpair_t *parent); Modified: user/ngie/stable-10-libnv/sys/contrib/libnv/nvlist.c ============================================================================== --- user/ngie/stable-10-libnv/sys/contrib/libnv/nvlist.c Sun Jan 3 08:48:23 2016 (r293091) +++ user/ngie/stable-10-libnv/sys/contrib/libnv/nvlist.c Sun Jan 3 09:08:34 2016 (r293092) @@ -330,7 +330,7 @@ nvlist_clone(const nvlist_t *nvl) newnvp = nvpair_clone(nvp); if (newnvp == NULL) break; - nvlist_move_nvpair(newnvl, newnvp); + (void)nvlist_move_nvpair(newnvl, newnvp); } if (nvp != NULL) { nvlist_destroy(newnvl); @@ -629,10 +629,8 @@ nvlist_xpack(const nvlist_t *nvl, int64_ nvpair_init_datasize(nvp); ptr = nvpair_pack_header(nvp, ptr, &left); - if (ptr == NULL) { - nv_free(buf); - return (NULL); - } + if (ptr == NULL) + goto fail; switch (nvpair_type(nvp)) { case NV_TYPE_NULL: ptr = nvpair_pack_null(nvp, ptr, &left); @@ -650,7 +648,7 @@ nvlist_xpack(const nvlist_t *nvl, int64_ tmpnvl = nvpair_get_nvlist(nvp); ptr = nvlist_pack_header(tmpnvl, ptr, &left); if (ptr == NULL) - goto out; + goto fail; tmpnvp = nvlist_first_nvpair(tmpnvl); if (tmpnvp != NULL) { nvl = tmpnvl; @@ -670,10 +668,8 @@ nvlist_xpack(const nvlist_t *nvl, int64_ default: PJDLOG_ABORT("Invalid type (%d).", nvpair_type(nvp)); } - if (ptr == NULL) { - nv_free(buf); - return (NULL); - } + if (ptr == NULL) + goto fail; while ((nvp = nvlist_next_nvpair(nvl, nvp)) == NULL) { cookie = NULL; nvl = nvlist_get_parent(nvl, &cookie); @@ -682,7 +678,7 @@ nvlist_xpack(const nvlist_t *nvl, int64_ nvp = cookie; ptr = nvpair_pack_nvlist_up(ptr, &left); if (ptr == NULL) - goto out; + goto fail; } } @@ -690,6 +686,9 @@ out: if (sizep != NULL) *sizep = size; return (buf); +fail: + nv_free(buf); + return (NULL); } void * @@ -824,6 +823,8 @@ nvlist_xunpack(const void *buf, size_t s case NV_TYPE_NVLIST: ptr = nvpair_unpack_nvlist(isbe, nvp, ptr, &left, nfds, &tmpnvl); + if (tmpnvl == NULL || ptr == NULL) + goto failed; nvlist_set_parent(tmpnvl, nvp); break; #ifndef _KERNEL @@ -846,7 +847,8 @@ nvlist_xunpack(const void *buf, size_t s } if (ptr == NULL) goto failed; - nvlist_move_nvpair(nvl, nvp); + if (!nvlist_move_nvpair(nvl, nvp)) + goto failed; if (tmpnvl != NULL) { nvl = tmpnvl; tmpnvl = NULL; @@ -1026,9 +1028,8 @@ nvlist_next(const nvlist_t *nvl, int *ty nvpair_t *nvp; NVLIST_ASSERT(nvl); - PJDLOG_ASSERT(cookiep != NULL); - if (*cookiep == NULL) + if (cookiep == NULL || *cookiep == NULL) nvp = nvlist_first_nvpair(nvl); else nvp = nvlist_next_nvpair(nvl, *cookiep); @@ -1036,7 +1037,8 @@ nvlist_next(const nvlist_t *nvl, int *ty return (NULL); if (typep != NULL) *typep = nvpair_type(nvp); - *cookiep = nvp; + if (cookiep != NULL) + *cookiep = nvp; return (nvpair_name(nvp)); } @@ -1122,7 +1124,7 @@ nvlist_add_stringv(nvlist_t *nvl, const nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } @@ -1141,7 +1143,7 @@ nvlist_add_null(nvlist_t *nvl, const cha nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } @@ -1161,7 +1163,7 @@ nvlist_add_binary(nvlist_t *nvl, const c nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } @@ -1182,7 +1184,7 @@ nvlist_add_##type(nvlist_t *nvl, const c nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); \ ERRNO_SET(nvl->nvl_error); \ } else { \ - nvlist_move_nvpair(nvl, nvp); \ + (void)nvlist_move_nvpair(nvl, nvp); \ } \ } @@ -1196,7 +1198,7 @@ NVLIST_ADD(int, descriptor); #undef NVLIST_ADD -void +bool nvlist_move_nvpair(nvlist_t *nvl, nvpair_t *nvp) { @@ -1206,18 +1208,19 @@ nvlist_move_nvpair(nvlist_t *nvl, nvpair if (nvlist_error(nvl) != 0) { nvpair_free(nvp); ERRNO_SET(nvlist_error(nvl)); - return; + return (false); } if ((nvl->nvl_flags & NV_FLAG_NO_UNIQUE) == 0) { if (nvlist_exists(nvl, nvpair_name(nvp))) { nvpair_free(nvp); nvl->nvl_error = EEXIST; ERRNO_SET(nvl->nvl_error); - return; + return (false); } } nvpair_insert(&nvl->nvl_head, nvp, nvl); + return (true); } void @@ -1236,7 +1239,7 @@ nvlist_move_string(nvlist_t *nvl, const nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } @@ -1257,7 +1260,7 @@ nvlist_move_nvlist(nvlist_t *nvl, const nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } @@ -1278,7 +1281,7 @@ nvlist_move_descriptor(nvlist_t *nvl, co nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } } #endif @@ -1299,7 +1302,7 @@ nvlist_move_binary(nvlist_t *nvl, const nvl->nvl_error = ERRNO_OR_DEFAULT(ENOMEM); ERRNO_SET(nvl->nvl_error); } else { - nvlist_move_nvpair(nvl, nvp); + (void)nvlist_move_nvpair(nvl, nvp); } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201601030908.u0398Yoc026138>