[Asterisk-code-review] jansson: Backport fixes to bundled, use json vsprintf if ava... (asterisk[master])

George Joseph asteriskteam at digium.com
Wed Sep 26 11:09:51 CDT 2018


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10255 )

Change subject: jansson: Backport fixes to bundled, use json_vsprintf if available.
......................................................................

jansson: Backport fixes to bundled, use json_vsprintf if available.

Use json_vsprintf from versions which contain fix for va_copy leak.

Apply fixes from jansson master:
* va_copy leak fix.
* Avoid potential invalid memory read in json_pack.
* Rename variable that shadowed another.

Change-Id: I7522e462d2a52f53010ffa1e7d705c666ec35539
---
M configure
M include/asterisk/autoconfig.h.in
M main/json.c
M menuselect/configure
M third-party/jansson/configure.m4
A third-party/jansson/patches/0022-Avoid-invalid-memory-read-in-json_pack.patch
A third-party/jansson/patches/0025-Call-va_end-after-va_copy-in-json_vsprintf.patch
A third-party/jansson/patches/0027-Rename-a-varialble-that-shadows-another-one.patch
8 files changed, 182 insertions(+), 34 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/configure b/configure
index 6c5d973..ac419e2 100755
--- a/configure
+++ b/configure
@@ -1337,7 +1337,6 @@
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -1525,7 +1524,6 @@
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1778,15 +1776,6 @@
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1924,7 +1913,7 @@
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -2077,7 +2066,6 @@
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
@@ -9305,6 +9293,9 @@
 	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
 $as_echo "yes" >&6; }
 
+$as_echo "#define HAVE_JANSSON_BUNDLED 1" >>confdefs.h
+
+
 	fi
 
 
@@ -14815,7 +14806,7 @@
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -14861,7 +14852,7 @@
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -14885,7 +14876,7 @@
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -14930,7 +14921,7 @@
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -14954,7 +14945,7 @@
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -16254,8 +16245,6 @@
     if (*(data + i) != *(data3 + i))
       return 14;
   close (fd);
-  free (data);
-  free (data3);
   return 0;
 }
 _ACEOF
diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index 69ab585..66c0ce4 100644
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -410,6 +410,9 @@
 /* Define if your system has the JANSSON libraries. */
 #undef HAVE_JANSSON
 
+/* Define if your system has JANSSON_BUNDLED */
+#undef HAVE_JANSSON_BUNDLED
+
 /* Define to 1 if you have the `kevent64' function. */
 #undef HAVE_KEVENT64
 
diff --git a/main/json.c b/main/json.c
index f8f4c0d..a9f6b42 100644
--- a/main/json.c
+++ b/main/json.c
@@ -292,16 +292,25 @@
 
 struct ast_json *ast_json_vstringf(const char *format, va_list args)
 {
-	char *str = NULL;
 	json_t *ret = NULL;
 
 	if (format) {
+		/* json_pack was not introduced until jansson-2.0 so Asterisk could never
+		 * be compiled against older versions.  The version check can never match
+		 * anything older than 2.12. */
+#if defined(HAVE_JANSSON_BUNDLED) || JANSSON_MAJOR_VERSION > 2 || JANSSON_MINOR_VERSION > 11
+		ret = json_vsprintf(format, args);
+#else
+		char *str = NULL;
 		int err = ast_vasprintf(&str, format, args);
+
 		if (err >= 0) {
 			ret = json_string(str);
 			ast_free(str);
 		}
+#endif
 	}
+
 	return (struct ast_json *)ret;
 }
 
diff --git a/menuselect/configure b/menuselect/configure
index fd7d24b..8efb637 100755
--- a/menuselect/configure
+++ b/menuselect/configure
@@ -692,7 +692,6 @@
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -773,7 +772,6 @@
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE}'
@@ -1026,15 +1024,6 @@
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1172,7 +1161,7 @@
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1325,7 +1314,6 @@
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
diff --git a/third-party/jansson/configure.m4 b/third-party/jansson/configure.m4
index d59d861..aa112f0 100644
--- a/third-party/jansson/configure.m4
+++ b/third-party/jansson/configure.m4
@@ -79,6 +79,7 @@
 	AC_SUBST([JANSSON_LIB])
 	AC_SUBST([JANSSON_INCLUDE])
 	AC_MSG_RESULT(yes)
+	AC_DEFINE([HAVE_JANSSON_BUNDLED], 1, [Define if your system has JANSSON_BUNDLED])
 ])
 
 AC_DEFUN([JANSSON_CONFIGURE],
diff --git a/third-party/jansson/patches/0022-Avoid-invalid-memory-read-in-json_pack.patch b/third-party/jansson/patches/0022-Avoid-invalid-memory-read-in-json_pack.patch
new file mode 100644
index 0000000..2e8f746
--- /dev/null
+++ b/third-party/jansson/patches/0022-Avoid-invalid-memory-read-in-json_pack.patch
@@ -0,0 +1,38 @@
+From aed855e6920923898b94a1b922fbace27a34ddf2 Mon Sep 17 00:00:00 2001
+From: Petri Lehtinen <petri at digip.org>
+Date: Mon, 9 Jul 2018 22:26:35 +0300
+Subject: [PATCH 22/29] Avoid invalid memory read in json_pack()
+
+Initial patch by @bharjoc-bitdefender
+
+Fixes #421
+---
+ src/pack_unpack.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/src/pack_unpack.c b/src/pack_unpack.c
+index 6461c06..b842772 100644
+--- a/src/pack_unpack.c
++++ b/src/pack_unpack.c
+@@ -75,6 +75,9 @@ static void next_token(scanner_t *s)
+         return;
+     }
+ 
++    if (!token(s) && !*s->fmt)
++        return;
++
+     t = s->fmt;
+     s->column++;
+     s->pos++;
+@@ -97,7 +100,7 @@ static void next_token(scanner_t *s)
+     s->token.column = s->column;
+     s->token.pos = s->pos;
+ 
+-    t++;
++    if (*t) t++;
+     s->fmt = t;
+ }
+ 
+-- 
+2.17.1
+
diff --git a/third-party/jansson/patches/0025-Call-va_end-after-va_copy-in-json_vsprintf.patch b/third-party/jansson/patches/0025-Call-va_end-after-va_copy-in-json_vsprintf.patch
new file mode 100644
index 0000000..a56c03c
--- /dev/null
+++ b/third-party/jansson/patches/0025-Call-va_end-after-va_copy-in-json_vsprintf.patch
@@ -0,0 +1,64 @@
+From 66e4ee795d21a30118f8503c966e9f9ae87db315 Mon Sep 17 00:00:00 2001
+From: Xin Long <lucien.xin at gmail.com>
+Date: Wed, 25 Jul 2018 17:39:33 +0800
+Subject: [PATCH 25/29] Call va_end after va_copy in json_vsprintf
+
+As said in man doc:
+  "Each  invocation  of va_copy() must be matched by a corresponding
+   invocation of va_end() in the same function."
+
+va_copy may alloc memory in some system, it's necessay to free it by
+va_end.
+
+Fixes: efe6c7b3f2b3 ("Add json_sprintf and json_vsprintf")
+Signed-off-by: Xin Long <lucien.xin at gmail.com>
+---
+ src/value.c | 17 ++++++++++++-----
+ 1 file changed, 12 insertions(+), 5 deletions(-)
+
+diff --git a/src/value.c b/src/value.c
+index 29a978c..861dce8 100644
+--- a/src/value.c
++++ b/src/value.c
+@@ -781,26 +781,33 @@ static json_t *json_string_copy(const json_t *string)
+ }
+ 
+ json_t *json_vsprintf(const char *fmt, va_list ap) {
++    json_t *json = NULL;
+     int length;
+     char *buf;
+     va_list aq;
+     va_copy(aq, ap);
+ 
+     length = vsnprintf(NULL, 0, fmt, ap);
+-    if (length == 0)
+-        return json_string("");
++    if (length == 0) {
++        json = json_string("");
++        goto out;
++    }
+ 
+     buf = jsonp_malloc(length + 1);
+     if (!buf)
+-        return NULL;
++        goto out;
+ 
+     vsnprintf(buf, length + 1, fmt, aq);
+     if (!utf8_check_string(buf, length)) {
+         jsonp_free(buf);
+-        return NULL;
++        goto out;
+     }
+ 
+-    return jsonp_stringn_nocheck_own(buf, length);
++    json = jsonp_stringn_nocheck_own(buf, length);
++
++out:
++    va_end(aq);
++    return json;
+ }
+ 
+ json_t *json_sprintf(const char *fmt, ...) {
+-- 
+2.17.1
+
diff --git a/third-party/jansson/patches/0027-Rename-a-varialble-that-shadows-another-one.patch b/third-party/jansson/patches/0027-Rename-a-varialble-that-shadows-another-one.patch
new file mode 100644
index 0000000..fc4ce07
--- /dev/null
+++ b/third-party/jansson/patches/0027-Rename-a-varialble-that-shadows-another-one.patch
@@ -0,0 +1,56 @@
+From 020cc26b5cb147ae3569a3f7d314d3900b4bbc0b Mon Sep 17 00:00:00 2001
+From: Petri Lehtinen <petri at digip.org>
+Date: Sun, 12 Aug 2018 18:25:51 +0300
+Subject: [PATCH 27/29] Rename a varialble that shadows another one
+
+configure.ac changes are removed for bundled jansson.
+
+Fixes #430
+---
+ configure.ac | 2 +-
+ src/dump.c   | 8 ++++----
+ 2 files changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/src/dump.c b/src/dump.c
+index 8e725c9..4a64aa4 100644
+--- a/src/dump.c
++++ b/src/dump.c
+@@ -306,7 +306,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
+             const char *separator;
+             int separator_length;
+             /* Space for "0x", double the sizeof a pointer for the hex and a terminator. */
+-            char key[2 + (sizeof(json) * 2) + 1];
++            char loop_key[2 + (sizeof(json) * 2) + 1];
+ 
+             if(flags & JSON_COMPACT) {
+                 separator = ":";
+@@ -318,7 +318,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
+             }
+ 
+             /* detect circular references */
+-            if (loop_check(parents, json, key, sizeof(key)))
++            if (loop_check(parents, json, loop_key, sizeof(loop_key)))
+                 return -1;
+ 
+             iter = json_object_iter((json_t *)json);
+@@ -326,7 +326,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
+             if(!embed && dump("{", 1, data))
+                 return -1;
+             if(!iter) {
+-                hashtable_del(parents, key);
++                hashtable_del(parents, loop_key);
+                 return embed ? 0 : dump("}", 1, data);
+             }
+             if(dump_indent(flags, depth + 1, 0, dump, data))
+@@ -422,7 +422,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
+                 }
+             }
+ 
+-            hashtable_del(parents, key);
++            hashtable_del(parents, loop_key);
+             return embed ? 0 : dump("}", 1, data);
+         }
+ 
+-- 
+2.17.1
+

-- 
To view, visit https://gerrit.asterisk.org/10255
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7522e462d2a52f53010ffa1e7d705c666ec35539
Gerrit-Change-Number: 10255
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180926/59bca500/attachment-0001.html>


More information about the asterisk-code-review mailing list