[PATCH for-4.21 0/6] tools/{lib,}xl: Coverity falllout from the json-c switch

[PATCH for-4.21 0/6] tools/{lib,}xl: Coverity falllout from the json-c switch

[Roger Pau Monne]

Hello,

The following series contains fixes for Coverity reported issues after
the switch from YAJL to json-c.  Those reports are from the internal
XenServer Coverity instance, as the project one hasn't been switched to
use json-c yet, last patch in the series switches thegithub triggered
Coverity build to use json-c.

All should be considered for 4.21.

Thanks, Roger.

Roger Pau Monne (6):
  tools/{lib,}xl: fix usage of error return from
    json_tokener_parse_verbose()
  tools/libxl: avoid freeing stack rubble in
    libxl__json_object_to_json()
  tools/xl: check return of json_object_object_add()
  tools/xl: check return value of printf_info_one_json() in
    list_domains_details()
  tools/xl: fix possible uninitialized usage in printf_info()
  github/coverity: switch to building with json-c instead of yajl

 .github/workflows/coverity.yml |  2 +-
 tools/libs/light/libxl_json.c  |  8 +++++---
 tools/xl/xl_info.c             | 22 +++++++++++++++-------
 3 files changed, 21 insertions(+), 11 deletions(-)

-- 
2.51.0

[PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()

[Roger Pau Monne]

It's possible for json_tokener_parse_verbose() to return NULL and leave the
error parameter unset.  Initialize the error token to success, and only
print it if the function has actually set it to a value different than
success.

Reported by XenServer internal Coverity instance.

Fixes: 7e95dab9eb63 ("libxl: Convert libxl__json_parse() to use json-c")
Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libs/light/libxl_json.c | 6 ++++--
 tools/xl/xl_info.c            | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
index c76ae9f64a9d..a9e06b06932d 100644
--- a/tools/libs/light/libxl_json.c
+++ b/tools/libs/light/libxl_json.c
@@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
     libxl__json_object *o = NULL;
 #ifdef USE_LIBJSONC_PARSER
     json_object *jso;
-    enum json_tokener_error error;
+    enum json_tokener_error error = json_tokener_success;
 
     jso = json_tokener_parse_verbose(s, &error);
     if (!jso) {
-        LOG(ERROR, "json-c parse error: %s", json_tokener_error_desc(error));
+        LOG(ERROR, "json-c parse error: %s",
+            error != json_tokener_success ? json_tokener_error_desc(error)
+                                          : "unspecified error");
         goto out;
     }
 #endif
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 80a3b25aac81..777ff2c64294 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -67,7 +67,7 @@ static int printf_info_one_json(json_object **jso_r, int domid,
 {
     json_object *jso = NULL;
     json_object *jso_config = NULL;
-    enum json_tokener_error error;
+    enum json_tokener_error error = json_tokener_success;
     char *s = NULL;
     int r = EXIT_FAILURE;
 
@@ -75,7 +75,8 @@ static int printf_info_one_json(json_object **jso_r, int domid,
     jso_config = json_tokener_parse_verbose(s, &error);
     if (!jso_config) {
         fprintf(stderr, "fail to parse JSON from libxl_domain_config_to_json(): %s\n",
-                json_tokener_error_desc(error));
+                error != json_tokener_success ? json_tokener_error_desc(error)
+                                              : "unspecified error");
         goto out;
     }
 
-- 
2.51.0

[PATCH for-4.21 2/6] tools/libxl: avoid freeing stack rubble in libxl__json_object_to_json()

[Roger Pau Monne]

It's possible for libxl__json_object_to_json_object() to not set the passed
jso_out parameter, hence initialize it in libxl__json_object_to_json() to
avoid freeing an uninitialized pointer in case of failure.

Reported by XenServer internal Coverity instance.

Fixes: 75fa670e582c ("libxl: Convert libxl__json_object_to_json() to json_object")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libs/light/libxl_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
index a9e06b06932d..b01d86363550 100644
--- a/tools/libs/light/libxl_json.c
+++ b/tools/libs/light/libxl_json.c
@@ -1533,7 +1533,7 @@ char *libxl__json_object_to_json(libxl__gc *gc,
 {
 #ifdef HAVE_LIBJSONC
     const char *buf;
-    json_object *root;
+    json_object *root = NULL;
     char *ret = NULL;
     int rc;
 
-- 
2.51.0

[PATCH for-4.21 3/6] tools/xl: check return of json_object_object_add()

[Roger Pau Monne]

Check the return codes of json_object_object_add() calls in
printf_info_one_json().

Reported by XenServer internal Coverity instance.

Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/xl/xl_info.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 777ff2c64294..2048669abbd4 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -69,7 +69,7 @@ static int printf_info_one_json(json_object **jso_r, int domid,
     json_object *jso_config = NULL;
     enum json_tokener_error error = json_tokener_success;
     char *s = NULL;
-    int r = EXIT_FAILURE;
+    int r = EXIT_FAILURE, rc;
 
     s = libxl_domain_config_to_json(ctx, d_config);
     jso_config = json_tokener_parse_verbose(s, &error);
@@ -82,12 +82,17 @@ static int printf_info_one_json(json_object **jso_r, int domid,
 
     jso = json_object_new_object();
     if (domid != -1)
-        json_object_object_add(jso, "domid", json_object_new_int(domid));
+        rc = json_object_object_add(jso, "domid", json_object_new_int(domid));
     else
-        json_object_object_add(jso, "domid", json_object_new_null());
+        rc = json_object_object_add(jso, "domid", json_object_new_null());
+    if (rc)
+        goto out;
+
 
+    rc = json_object_object_add(jso, "config", jso_config);
+    if (rc)
+        goto out;
 
-    json_object_object_add(jso, "config", jso_config);
     jso_config = NULL;
 
     *jso_r = jso;
-- 
2.51.0

[PATCH for-4.21 4/6] tools/xl: check return value of printf_info_one_json() in list_domains_details()

[Roger Pau Monne]

And return from the function early if printf_info_one_json() returns error.

Reported by internal XenServer Coverity instance.

Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Roger Pau Monné <roger.pua@citrix.com>
---
 tools/xl/xl_info.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 2048669abbd4..696f1f7423cb 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -585,6 +585,8 @@ static void list_domains_details(const libxl_dominfo *info, int nb_domain)
 #ifdef HAVE_LIBJSONC
             json_object *jso_value;
             rc = printf_info_one_json(&jso_value, info[i].domid, &d_config);
+            if (rc)
+                goto out;
             json_object_array_add(jso, jso_value);
 #elif defined(HAVE_LIBYAJL)
             s = printf_info_one_json(hand, info[i].domid, &d_config);
-- 
2.51.0

[PATCH for-4.21 5/6] tools/xl: fix possible uninitialized usage in printf_info()

[Roger Pau Monne]

printf_info_one_json() won't initialize the passed jso_r parameter on
error, and hence the usage in printf_info() needs prior initialization,
otherwise an uninitialized pointer is passed to json_object_put() on
failure.

Reported by the internal XenServer Coverity instance.

Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/xl/xl_info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 696f1f7423cb..0314ce7d4c22 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -155,7 +155,7 @@ void printf_info(enum output_format output_format,
 #ifdef HAVE_LIBJSONC
     int r;
     const char *buf;
-    json_object *jso;
+    json_object *jso = NULL;
 
     r = printf_info_one_json(&jso, domid, d_config);
     if (r)
-- 
2.51.0

[PATCH for-4.21 6/6] github/coverity: switch to building with json-c instead of yajl

[Roger Pau Monne]

Switch the json library used by the Coverity runs.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 .github/workflows/coverity.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index a6c2819b0a24..a9a971f6a1e5 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -20,9 +20,9 @@ jobs:
           iasl \
           libbz2-dev \
           libext2fs-dev \
+          libjson-c-dev \
           liblzma-dev \
           libncurses5-dev \
-          libyajl-dev \
           libzstd-dev \
           ocaml \
           ocaml-findlib \
-- 
2.51.0

Re: [PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()

[Andrew Cooper]

On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
> index c76ae9f64a9d..a9e06b06932d 100644
> --- a/tools/libs/light/libxl_json.c
> +++ b/tools/libs/light/libxl_json.c
> @@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
>      libxl__json_object *o = NULL;
>  #ifdef USE_LIBJSONC_PARSER
>      json_object *jso;
> -    enum json_tokener_error error;
> +    enum json_tokener_error error = json_tokener_success;

Looking at the options available, I'd suggest initialising to:

    json_tokener_error_parse_unexpected

and dropping the rest of the hunk.  I wouldn't assume that success
cannot be passed here.

~Andrew

>  
>      jso = json_tokener_parse_verbose(s, &error);
>      if (!jso) {
> -        LOG(ERROR, "json-c parse error: %s", json_tokener_error_desc(error));
> +        LOG(ERROR, "json-c parse error: %s",
> +            error != json_tokener_success ? json_tokener_error_desc(error)
> +                                          : "unspecified error");
>          goto out;
>      }
>  #endif

Re: [PATCH for-4.21 1/6] tools/{lib,}xl: fix usage of error return from json_tokener_parse_verbose()

[Roger Pau Monné]

On Wed, Oct 15, 2025 at 02:59:25PM +0100, Andrew Cooper wrote:
> On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> > diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
> > index c76ae9f64a9d..a9e06b06932d 100644
> > --- a/tools/libs/light/libxl_json.c
> > +++ b/tools/libs/light/libxl_json.c
> > @@ -1366,11 +1366,13 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
> >      libxl__json_object *o = NULL;
> >  #ifdef USE_LIBJSONC_PARSER
> >      json_object *jso;
> > -    enum json_tokener_error error;
> > +    enum json_tokener_error error = json_tokener_success;
> 
> Looking at the options available, I'd suggest initialising to:
> 
>     json_tokener_error_parse_unexpected
> 
> and dropping the rest of the hunk.  I wouldn't assume that success
> cannot be passed here.

That error code translates to "unexpected character", which I didn't
think was very accurate here.  I didn't find any good error code to
map here, hence why I went with this kind of weird solution.

I don't mind using json_tokener_error_parse_unexpected, just
mentioning why I didn't use it in the first place.  Anthony, what's
your opinion?

Thanks, Roger.

Re: [PATCH for-4.21 0/6] tools/{lib,}xl: Coverity falllout from the json-c switch

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]


On 10/15/25 3:40 PM, Roger Pau Monne wrote:
> Hello,
>
> The following series contains fixes for Coverity reported issues after
> the switch from YAJL to json-c.  Those reports are from the internal
> XenServer Coverity instance, as the project one hasn't been switched to
> use json-c yet, last patch in the series switches thegithub triggered
> Coverity build to use json-c.
>
> All should be considered for 4.21.

Agree, we could consider that to have that in 4.21 as it fixes some minor
problems and isn't too intrusive:
  Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

>
> Thanks, Roger.
>
> Roger Pau Monne (6):
>    tools/{lib,}xl: fix usage of error return from
>      json_tokener_parse_verbose()
>    tools/libxl: avoid freeing stack rubble in
>      libxl__json_object_to_json()
>    tools/xl: check return of json_object_object_add()
>    tools/xl: check return value of printf_info_one_json() in
>      list_domains_details()
>    tools/xl: fix possible uninitialized usage in printf_info()
>    github/coverity: switch to building with json-c instead of yajl
>
>   .github/workflows/coverity.yml |  2 +-
>   tools/libs/light/libxl_json.c  |  8 +++++---
>   tools/xl/xl_info.c             | 22 +++++++++++++++-------
>   3 files changed, 21 insertions(+), 11 deletions(-)
>

[-- Attachment #2: Type: text/html, Size: 1886 bytes --]

Re: [PATCH for-4.21 0/6] tools/{lib,}xl: Coverity falllout from the json-c switch

[Andrew Cooper]

On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> Hello,
>
> The following series contains fixes for Coverity reported issues after
> the switch from YAJL to json-c.  Those reports are from the internal
> XenServer Coverity instance, as the project one hasn't been switched to
> use json-c yet, last patch in the series switches thegithub triggered
> Coverity build to use json-c.
>
> All should be considered for 4.21.
>
> Thanks, Roger.
>
> Roger Pau Monne (6):
>   tools/{lib,}xl: fix usage of error return from
>     json_tokener_parse_verbose()
>   tools/libxl: avoid freeing stack rubble in
>     libxl__json_object_to_json()
>   tools/xl: check return of json_object_object_add()
>   tools/xl: check return value of printf_info_one_json() in
>     list_domains_details()
>   tools/xl: fix possible uninitialized usage in printf_info()
>   github/coverity: switch to building with json-c instead of yajl

Other than my query on patch 1, everything else LGTM.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thankyou for looking at these.

Re: [PATCH for-4.21 0/6] tools/{lib,}xl: Coverity falllout from the json-c switch

[Roger Pau Monné]

On Wed, Oct 15, 2025 at 04:04:06PM +0100, Andrew Cooper wrote:
> On 15/10/2025 2:40 pm, Roger Pau Monne wrote:
> > Hello,
> >
> > The following series contains fixes for Coverity reported issues after
> > the switch from YAJL to json-c.  Those reports are from the internal
> > XenServer Coverity instance, as the project one hasn't been switched to
> > use json-c yet, last patch in the series switches thegithub triggered
> > Coverity build to use json-c.
> >
> > All should be considered for 4.21.
> >
> > Thanks, Roger.
> >
> > Roger Pau Monne (6):
> >   tools/{lib,}xl: fix usage of error return from
> >     json_tokener_parse_verbose()
> >   tools/libxl: avoid freeing stack rubble in
> >     libxl__json_object_to_json()
> >   tools/xl: check return of json_object_object_add()
> >   tools/xl: check return value of printf_info_one_json() in
> >     list_domains_details()
> >   tools/xl: fix possible uninitialized usage in printf_info()
> >   github/coverity: switch to building with json-c instead of yajl
> 
> Other than my query on patch 1, everything else LGTM.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks, if no one else expresses an opinion by tomorrow I will apply
your comment and commit.

Roger.

Re: [PATCH for-4.21 2/6] tools/libxl: avoid freeing stack rubble in libxl__json_object_to_json()

[Jason Andryuk]

On 2025-10-15 09:40, Roger Pau Monne wrote:
> It's possible for libxl__json_object_to_json_object() to not set the passed
> jso_out parameter, hence initialize it in libxl__json_object_to_json() to
> avoid freeing an uninitialized pointer in case of failure.
> 
> Reported by XenServer internal Coverity instance.
> 
> Fixes: 75fa670e582c ("libxl: Convert libxl__json_object_to_json() to json_object")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH for-4.21 3/6] tools/xl: check return of json_object_object_add()

[Jason Andryuk]

On 2025-10-15 09:40, Roger Pau Monne wrote:
> Check the return codes of json_object_object_add() calls in
> printf_info_one_json().
> 
> Reported by XenServer internal Coverity instance.
> 
> Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH for-4.21 4/6] tools/xl: check return value of printf_info_one_json() in list_domains_details()

[Jason Andryuk]

On 2025-10-15 09:40, Roger Pau Monne wrote:
> And return from the function early if printf_info_one_json() returns error.
> 
> Reported by internal XenServer Coverity instance.
> 
> Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
> Signed-off-by: Roger Pau Monné <roger.pua@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH for-4.21 5/6] tools/xl: fix possible uninitialized usage in printf_info()

[Jason Andryuk]

On 2025-10-15 09:40, Roger Pau Monne wrote:
> printf_info_one_json() won't initialize the passed jso_r parameter on
> error, and hence the usage in printf_info() needs prior initialization,
> otherwise an uninitialized pointer is passed to json_object_put() on
> failure.
> 
> Reported by the internal XenServer Coverity instance.
> 
> Fixes: f6c6f2679d49 ("libxl: Convert libxl__object_to_json() to json-c")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Re: [PATCH for-4.21 6/6] github/coverity: switch to building with json-c instead of yajl

[Jason Andryuk]

On 2025-10-15 09:40, Roger Pau Monne wrote:
> Switch the json library used by the Coverity runs.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>