mailarchive of the ptxdist mailing list
 help / color / mirror / Atom feed
* [ptxdist] [PATCH] json-c: Add patches with memory fixes from upstream master
@ 2026-07-16 13:27 Alexander Dahl via ptxdist
  2026-08-14 15:38 ` [ptxdist] [APPLIED] " Michael Olbrich
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Dahl via ptxdist @ 2026-07-16 13:27 UTC (permalink / raw)
  To: ptxdist; +Cc: Alexander Dahl

Fixing memory leak and reference counting bugs, no new release yet.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 ...e-to-clear-_user_delete-in-_json_obj.patch |  96 +++++++++
 ...ts-outside-assert-in-json_object_put.patch |  31 +++
 ...et-_userdata-NULL-in-_json_object_ma.patch |  47 +++++
 ...put-to-return-1-for-freed-scalars-an.patch | 195 ++++++++++++++++++
 patches/json-c-0.19/series                    |   7 +
 5 files changed, 376 insertions(+)
 create mode 100644 patches/json-c-0.19/0001-Issue-934-Be-sure-to-clear-_user_delete-in-_json_obj.patch
 create mode 100644 patches/json-c-0.19/0002-free-objects-outside-assert-in-json_object_put.patch
 create mode 100644 patches/json-c-0.19/0003-Issue-934-Also-set-_userdata-NULL-in-_json_object_ma.patch
 create mode 100644 patches/json-c-0.19/0004-fix-json_object_put-to-return-1-for-freed-scalars-an.patch
 create mode 100644 patches/json-c-0.19/series

diff --git a/patches/json-c-0.19/0001-Issue-934-Be-sure-to-clear-_user_delete-in-_json_obj.patch b/patches/json-c-0.19/0001-Issue-934-Be-sure-to-clear-_user_delete-in-_json_obj.patch
new file mode 100644
index 000000000..9d30da84e
--- /dev/null
+++ b/patches/json-c-0.19/0001-Issue-934-Be-sure-to-clear-_user_delete-in-_json_obj.patch
@@ -0,0 +1,96 @@
+From: Eric Hawicz <erh+git@nimenees.com>
+Date: Sat, 4 Jul 2026 11:02:06 -0400
+Subject: [PATCH] Issue #934: Be sure to clear _user_delete in
+ _json_object_put_maybe_free() to avoid calling it twice. Add a test to check.
+
+Upstream-Status: Backport [3aa996b5b6bd96c29f2c6da98da33c3fb3d13038]
+---
+ json_object.c             |  1 +
+ json_object.h             |  4 ++--
+ tests/test_deep_nesting.c | 33 +++++++++++++++++++++++++++++++++
+ 3 files changed, 36 insertions(+), 2 deletions(-)
+
+diff --git a/json_object.c b/json_object.c
+index 0a619679bd7e..6cb1bdac86d5 100644
+--- a/json_object.c
++++ b/json_object.c
+@@ -303,6 +303,7 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
+ 
+ 	if (jso->_user_delete)
+ 		jso->_user_delete(jso, jso->_userdata);
++	jso->_user_delete = NULL;
+ 
+ 	switch (jso->o_type)
+ 	{
+diff --git a/json_object.h b/json_object.h
+index e21138ecf1db..7edb63d4f8cb 100644
+--- a/json_object.h
++++ b/json_object.h
+@@ -259,7 +259,7 @@ JSON_EXPORT void *json_object_get_userdata(json_object *jso);
+  * The user_delete parameter is optional and may be passed as NULL, even if
+  * the userdata parameter is non-NULL.  It will be called just before the
+  * json_object is deleted, after it's reference count goes to zero
+- * (see json_object_put()).
++ * (see json_object_put()) but before any child objects are freed.
+  * If this is not provided, it is up to the caller to free the userdata at
+  * an appropriate time. (i.e. after the json_object is deleted)
+  *
+@@ -293,7 +293,7 @@ JSON_EXPORT void json_object_set_userdata(json_object *jso, void *userdata,
+  * The user_delete parameter is optional and may be passed as NULL, even if
+  * the userdata parameter is non-NULL.  It will be called just before the
+  * json_object is deleted, after it's reference count goes to zero
+- * (see json_object_put()).
++ * (see json_object_put()) but before any child objects are freed.
+  * If this is not provided, it is up to the caller to free the userdata at
+  * an appropriate time. (i.e. after the json_object is deleted)
+  *
+diff --git a/tests/test_deep_nesting.c b/tests/test_deep_nesting.c
+index 364c4820ea5d..9e251750c480 100644
+--- a/tests/test_deep_nesting.c
++++ b/tests/test_deep_nesting.c
+@@ -56,6 +56,37 @@ static void test_deep_nesting_tostring(const char *str)
+ 	json_tokener_free(tok);
+ }
+ 
++struct userdata_test {
++	int userdata_val;
++	char *p;
++};
++/*
++ * Check that the user_delete function is only called once, even with the
++ * newer code to avoid deeply nested calls during json_object_put().
++ */
++static void user_delete_test(struct json_object *jso, void *userdata_in)
++{
++	struct userdata_test *userdata = (struct userdata_test *)userdata_in;
++	printf("in user_delete, userdata_val=%d\n", userdata->userdata_val);
++	fflush(stdout);
++	userdata->userdata_val = 0;
++	userdata->p[0] = 'x';
++	userdata->p[8191] = 'x';
++	free(userdata->p);
++}
++static void test_nesting_with_user_delete(void)
++{
++	json_object *jso;
++	struct userdata_test userdata_val = {
++		1, malloc(8192)
++	};
++
++ 	jso = json_object_new_object();
++	json_object_set_userdata(jso, &userdata_val, user_delete_test);
++	json_object_object_add(jso, "somekey", json_object_new_string("foo"));
++	json_object_put(jso);
++}
++
+ int main(int argc, char **argv)
+ {
+ 	char *str;	
+@@ -77,5 +108,7 @@ int main(int argc, char **argv)
+ 
+ 	free(str);
+ 
++	test_nesting_with_user_delete();
++
+ 	return EXIT_SUCCESS;
+ }
diff --git a/patches/json-c-0.19/0002-free-objects-outside-assert-in-json_object_put.patch b/patches/json-c-0.19/0002-free-objects-outside-assert-in-json_object_put.patch
new file mode 100644
index 000000000..7601bc68e
--- /dev/null
+++ b/patches/json-c-0.19/0002-free-objects-outside-assert-in-json_object_put.patch
@@ -0,0 +1,31 @@
+From: Javid Khan <dxbjavid@gmail.com>
+Date: Tue, 30 Jun 2026 18:54:06 +0530
+Subject: [PATCH] free objects outside assert() in json_object_put
+
+Upstream-Status: Backport [f291fa81c6f21d925ea771a8cca597f5886309cc]
+---
+ json_object.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/json_object.c b/json_object.c
+index 6cb1bdac86d5..71cd25b2a6a9 100644
+--- a/json_object.c
++++ b/json_object.c
+@@ -436,11 +436,16 @@ int json_object_put(struct json_object *jso)
+ 		// All slots are cleared, now pop back up to the parent
+ 		{
+ 			json_object *parent = jso->_delete_parent;
++			int rc;
+ 			// jso is a child that's already been detached from its parent
+ 			// so we need to actually free it now
+ 			assert(jso->_ref_count == 0);
+ 			jso->_ref_count++;   // We're the exclusive owner of jso, non-atomic add is ok.
+-			assert(_json_object_put_maybe_free(jso, 1) == 0);
++			// Note: the call must not be inside assert(), or it gets
++			// compiled out when NDEBUG is defined and the memory leaks.
++			rc = _json_object_put_maybe_free(jso, 1);
++			assert(rc == 0);
++			(void)rc;
+ 			jso = parent;
+ 			// iteration will be reset at the top of the loop
+ 		}
diff --git a/patches/json-c-0.19/0003-Issue-934-Also-set-_userdata-NULL-in-_json_object_ma.patch b/patches/json-c-0.19/0003-Issue-934-Also-set-_userdata-NULL-in-_json_object_ma.patch
new file mode 100644
index 000000000..b235b00bd
--- /dev/null
+++ b/patches/json-c-0.19/0003-Issue-934-Also-set-_userdata-NULL-in-_json_object_ma.patch
@@ -0,0 +1,47 @@
+From: Eric Hawicz <erh+git@nimenees.com>
+Date: Sat, 4 Jul 2026 11:38:15 -0400
+Subject: [PATCH] Issue #934: Also set _userdata=NULL in
+ _json_object_maybe_free() so json_object_put doesn't try to use it as a
+ _delete_parent value. Update the test_deep_nesting test output.
+
+Upstream-Status: Backport [0c3a5a1994ed6b5957d2e878fc1298ce9aa09b3f]
+---
+ json_object.c                    | 7 +++++--
+ tests/test_deep_nesting.expected | 1 +
+ 2 files changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/json_object.c b/json_object.c
+index 71cd25b2a6a9..095b84e2b4ab 100644
+--- a/json_object.c
++++ b/json_object.c
+@@ -304,6 +304,8 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
+ 	if (jso->_user_delete)
+ 		jso->_user_delete(jso, jso->_userdata);
+ 	jso->_user_delete = NULL;
++	jso->_userdata = NULL; // aka _delete_parent, but json_object_put() will
++	                       // have already grabbed it if it needs it.
+ 
+ 	switch (jso->o_type)
+ 	{
+@@ -435,10 +437,11 @@ int json_object_put(struct json_object *jso)
+ 
+ 		// All slots are cleared, now pop back up to the parent
+ 		{
+-			json_object *parent = jso->_delete_parent;
+-			int rc;
+ 			// jso is a child that's already been detached from its parent
+ 			// so we need to actually free it now
++			// Be sure to grab _delete_parent *before* freeing jso.
++			json_object *parent = jso->_delete_parent;
++			int rc;
+ 			assert(jso->_ref_count == 0);
+ 			jso->_ref_count++;   // We're the exclusive owner of jso, non-atomic add is ok.
+ 			// Note: the call must not be inside assert(), or it gets
+diff --git a/tests/test_deep_nesting.expected b/tests/test_deep_nesting.expected
+index b869cfaabe79..b9c1ab98480f 100644
+--- a/tests/test_deep_nesting.expected
++++ b/tests/test_deep_nesting.expected
+@@ -1,2 +1,3 @@
+ Parsed depth 100000 string to json_object: yes
+ Freed json_object
++in user_delete, userdata_val=1
diff --git a/patches/json-c-0.19/0004-fix-json_object_put-to-return-1-for-freed-scalars-an.patch b/patches/json-c-0.19/0004-fix-json_object_put-to-return-1-for-freed-scalars-an.patch
new file mode 100644
index 000000000..7e57bad29
--- /dev/null
+++ b/patches/json-c-0.19/0004-fix-json_object_put-to-return-1-for-freed-scalars-an.patch
@@ -0,0 +1,195 @@
+From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
+Date: Tue, 14 Jul 2026 09:32:03 -0400
+Subject: [PATCH] fix json_object_put() to return 1 for freed scalars and empty
+ containers
+
+The iterative rewrite in 17328a67 made json_object_put() return 0 for freed scalars, strings and empty containers, contradicting the documented contract in json_object.h. Distinguish "freed" from "still referenced" in _json_object_put_maybe_free() and add regression tests.
+
+Upstream-Status: Backport [69be99ef8e99d811d7810bdf75864ae800a67e50]
+---
+ json_object.c                    | 49 ++++++++++++++++++++++++++++------------
+ tests/test_deep_nesting.c        | 28 ++++++++++++++++++++++-
+ tests/test_deep_nesting.expected |  7 ++++++
+ 3 files changed, 69 insertions(+), 15 deletions(-)
+
+diff --git a/json_object.c b/json_object.c
+index 095b84e2b4ab..621260885620 100644
+--- a/json_object.c
++++ b/json_object.c
+@@ -273,13 +273,31 @@ struct json_object *json_object_get(struct json_object *jso)
+ }
+ 
+ 
++/**
++  * Return values for _json_object_put_maybe_free().
++  * json_object_put_still_refd and json_object_put_freed match the documented
++  * return values of json_object_put(), so they can be returned directly.
++  */
++enum json_object_put_result
++{
++	json_object_put_still_refd = 0, /* refcount decremented, object not freed */
++	json_object_put_freed = 1,      /* refcount reached zero, memory released */
++	json_object_put_container = 2   /* refcount reached zero, but the object is a
++	                                   non-empty container: the caller must free
++	                                   the contents, then the object itself */
++};
++
+ /**
+   * Internal json_object_put function
+-  * Returns 0 if we're done "freeing" the object, either because its memory
+-  * was actually released, or we just needed to decrement the refcount.
+-  * Returns 1 when the object is a non-empty container that still needs to be handled.
++  * Returns json_object_put_still_refd if a reference remains, and the object
++  * was not freed.
++  * Returns json_object_put_freed if the object's memory was released, either
++  * because it holds no other objects, or because free_containers was set.
++  * Returns json_object_put_container when the refcount reached zero but the
++  * object is a non-empty container; the caller must free the contents, then
++  * call this function again with free_containers set to free the object itself.
+   */
+-static inline int _json_object_put_maybe_free(struct json_object *jso, int free_containers)
++static inline enum json_object_put_result _json_object_put_maybe_free(struct json_object *jso, int free_containers)
+ {
+ 	/* Avoid invalid free and crash explicitly instead of (silently)
+ 	 * segfaulting.
+@@ -298,7 +316,7 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
+ 	if (--jso->_ref_count > 0)
+ #endif
+ 	{
+-		return 0;  // All done, caller doesn't need to do anything else
++		return json_object_put_still_refd;
+ 	}
+ 
+ 	if (jso->_user_delete)
+@@ -315,15 +333,15 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
+ 			json_object_object_delete(jso);
+ 			break;
+ 		}
+-		return 1;
+-	case json_type_array: 
++		return json_object_put_container;
++	case json_type_array:
+ 		// container objects are handled by the caller
+ 		if (free_containers || array_list_length(JC_ARRAY(jso)->c_array) == 0)
+ 		{
+ 			json_object_array_delete(jso);
+ 			break;
+ 		}
+-		return 1;
++		return json_object_put_container;
+ 	case json_type_string:
+ 		json_object_string_delete(jso);
+ 		break;
+@@ -331,16 +349,19 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
+ 		json_object_generic_delete(jso);
+ 		break;
+ 	}
+-	return 0;  // All done, caller doesn't need to do anything else
++	return json_object_put_freed;
+ }
+ 
+ int json_object_put(struct json_object *jso)
+ {
++	enum json_object_put_result rc;
++
+ 	if (!jso)
+ 		return 0;
+ 
+-	if (_json_object_put_maybe_free(jso, 0) == 0)
+-		return 0;
++	rc = _json_object_put_maybe_free(jso, 0);
++	if (rc != json_object_put_container)
++		return (int)rc;
+ 	// else, it's a non-empty container object, handle it below
+ 
+ 	// Note: jso is now a "zombie" object, _ref_count == 0 but memory not yet released
+@@ -399,7 +420,7 @@ int json_object_put(struct json_object *jso)
+ 			}
+ 
+ 			// Now, handle actually freeing the json_object in that slot
+-			if (!child || _json_object_put_maybe_free(child, 0) == 0)
++			if (!child || _json_object_put_maybe_free(child, 0) != json_object_put_container)
+ 			{
+  				// child is either freed, or still referenced somewhere else
+ 				// leave it as-is and handle the previous slot
+@@ -441,13 +462,13 @@ int json_object_put(struct json_object *jso)
+ 			// so we need to actually free it now
+ 			// Be sure to grab _delete_parent *before* freeing jso.
+ 			json_object *parent = jso->_delete_parent;
+-			int rc;
++			enum json_object_put_result rc;
+ 			assert(jso->_ref_count == 0);
+ 			jso->_ref_count++;   // We're the exclusive owner of jso, non-atomic add is ok.
+ 			// Note: the call must not be inside assert(), or it gets
+ 			// compiled out when NDEBUG is defined and the memory leaks.
+ 			rc = _json_object_put_maybe_free(jso, 1);
+-			assert(rc == 0);
++			assert(rc == json_object_put_freed);
+ 			(void)rc;
+ 			jso = parent;
+ 			// iteration will be reset at the top of the loop
+diff --git a/tests/test_deep_nesting.c b/tests/test_deep_nesting.c
+index 9e251750c480..4bf24578daa1 100644
+--- a/tests/test_deep_nesting.c
++++ b/tests/test_deep_nesting.c
+@@ -81,12 +81,36 @@ static void test_nesting_with_user_delete(void)
+ 		1, malloc(8192)
+ 	};
+ 
+- 	jso = json_object_new_object();
++	jso = json_object_new_object();
+ 	json_object_set_userdata(jso, &userdata_val, user_delete_test);
+ 	json_object_object_add(jso, "somekey", json_object_new_string("foo"));
+ 	json_object_put(jso);
+ }
+ 
++/*
++ * Check that json_object_put() returns 1 whenever the object is freed,
++ * including for scalars and empty containers, and 0 only when a
++ * reference remains.
++ */
++static void test_put_return_values(void)
++{
++	json_object *jso;
++
++	printf("put(empty object) returned %d\n", json_object_put(json_object_new_object()));
++	printf("put(empty array) returned %d\n", json_object_put(json_object_new_array()));
++	printf("put(string) returned %d\n", json_object_put(json_object_new_string("foo")));
++	printf("put(int) returned %d\n", json_object_put(json_object_new_int(42)));
++
++	jso = json_object_new_object();
++	json_object_object_add(jso, "somekey", json_object_new_string("foo"));
++	printf("put(non-empty object) returned %d\n", json_object_put(jso));
++
++	jso = json_object_new_object();
++	json_object_get(jso);
++	printf("put(still-referenced object) returned %d\n", json_object_put(jso));
++	printf("put(last reference) returned %d\n", json_object_put(jso));
++}
++
+ int main(int argc, char **argv)
+ {
+ 	char *str;	
+@@ -110,5 +134,7 @@ int main(int argc, char **argv)
+ 
+ 	test_nesting_with_user_delete();
+ 
++	test_put_return_values();
++
+ 	return EXIT_SUCCESS;
+ }
+diff --git a/tests/test_deep_nesting.expected b/tests/test_deep_nesting.expected
+index b9c1ab98480f..2449ede3265c 100644
+--- a/tests/test_deep_nesting.expected
++++ b/tests/test_deep_nesting.expected
+@@ -1,3 +1,10 @@
+ Parsed depth 100000 string to json_object: yes
+ Freed json_object
+ in user_delete, userdata_val=1
++put(empty object) returned 1
++put(empty array) returned 1
++put(string) returned 1
++put(int) returned 1
++put(non-empty object) returned 1
++put(still-referenced object) returned 0
++put(last reference) returned 1
diff --git a/patches/json-c-0.19/series b/patches/json-c-0.19/series
new file mode 100644
index 000000000..311299a3a
--- /dev/null
+++ b/patches/json-c-0.19/series
@@ -0,0 +1,7 @@
+# generated by git-ptx-patches
+#tag:base --start-number 1
+0001-Issue-934-Be-sure-to-clear-_user_delete-in-_json_obj.patch
+0002-free-objects-outside-assert-in-json_object_put.patch
+0003-Issue-934-Also-set-_userdata-NULL-in-_json_object_ma.patch
+0004-fix-json_object_put-to-return-1-for-freed-scalars-an.patch
+# 5aff3fd086034e7fb3fdec97d3808ade  - git-ptx-patches magic

base-commit: af7fe92c6398143ffbb7dfb75ae1e6de092cd15c
-- 
2.47.3




^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-14 15:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-16 13:27 [ptxdist] [PATCH] json-c: Add patches with memory fixes from upstream master Alexander Dahl via ptxdist
2026-08-14 15:38 ` [ptxdist] [APPLIED] " Michael Olbrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox