Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 11 Aug 2015 18:01:10 +0000 (UTC)
From:      Mariusz Zaborski <oshogbo@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r286645 - head/sys/contrib/libnv
Message-ID:  <201508111801.t7BI1A5T060408@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: oshogbo
Date: Tue Aug 11 18:01:10 2015
New Revision: 286645
URL: https://svnweb.freebsd.org/changeset/base/286645

Log:
  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.
  
  Approved by:	pjd (mentor)

Modified:
  head/sys/contrib/libnv/nv_impl.h
  head/sys/contrib/libnv/nvlist.c

Modified: head/sys/contrib/libnv/nv_impl.h
==============================================================================
--- head/sys/contrib/libnv/nv_impl.h	Tue Aug 11 17:54:51 2015	(r286644)
+++ head/sys/contrib/libnv/nv_impl.h	Tue Aug 11 18:01:10 2015	(r286645)
@@ -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: head/sys/contrib/libnv/nvlist.c
==============================================================================
--- head/sys/contrib/libnv/nvlist.c	Tue Aug 11 17:54:51 2015	(r286644)
+++ head/sys/contrib/libnv/nvlist.c	Tue Aug 11 18:01:10 2015	(r286645)
@@ -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);
@@ -848,7 +848,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;
@@ -1124,7 +1125,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);
 	}
 }
 
@@ -1143,7 +1144,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);
 	}
 }
 
@@ -1163,7 +1164,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);
 	}
 }
 
@@ -1184,7 +1185,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);			\
 	}								\
 }
 
@@ -1198,7 +1199,7 @@ NVLIST_ADD(int, descriptor);
 
 #undef	NVLIST_ADD
 
-void
+bool
 nvlist_move_nvpair(nvlist_t *nvl, nvpair_t *nvp)
 {
 
@@ -1208,18 +1209,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
@@ -1238,7 +1240,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);
 	}
 }
 
@@ -1259,7 +1261,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);
 	}
 }
 
@@ -1280,7 +1282,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
@@ -1301,7 +1303,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?201508111801.t7BI1A5T060408>