jsonb iterator not fully initialized

On 2018-05-26 02:02, Peter Eisentraut wrote:

I got this error message via -fsanitized=undefined:

jsonfuncs.c:5169:12: runtime error: load of value 127, which is not a
valid value for type '_Bool'

The query was

select ts_headline('{}'::jsonb, tsquery('aaa & bbb'));

This calls the C function ts_headline_jsonb_byid_opt(), which calls
transform_jsonb_string_values(), which calls

it = JsonbIteratorInit(&jsonb->root);
is_scalar = it->isScalar;

but it->isScalar is not always initialized by JsonbIteratorInit(). (So
the 127 is quite likely clobbered memory.)

It can be fixed this way:

--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
JsonbIterator *parent)
{
JsonbIterator *it;

- it = palloc(sizeof(JsonbIterator));
+ it = palloc0(sizeof(JsonbIterator));
it->container = container;
it->parent = parent;
it->nElems = JsonContainerSize(container);

It's probably not a problem in practice, since the isScalar business is
apparently only used in the array case, but it's dubious to leave things
uninitialized like this nonetheless.

I've seen it earlier but couldn't decide what my proposed fix should
look like. One of the options I considered was:

--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void 
*action_state,
         JsonbIteratorToken type;
         JsonbParseState *st = NULL;
         text       *out;
-       bool            is_scalar = false;

it = JsonbIteratorInit(&jsonb->root);
- is_scalar = it->isScalar;

while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
{
@@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
*action_state,
}

         if (res->type == jbvArray)
-               res->val.array.rawScalar = is_scalar;
+               res->val.array.rawScalar = 
JsonContainerIsScalar(&jsonb->root);

return JsonbValueToJsonb(res);
}

On 05/26/2018 03:09 AM, Piotr Stefaniak wrote:

On 2018-05-26 02:02, Peter Eisentraut wrote:

I got this error message via -fsanitized=undefined:

jsonfuncs.c:5169:12: runtime error: load of value 127, which is not a
valid value for type '_Bool'

The query was

select ts_headline('{}'::jsonb, tsquery('aaa & bbb'));

This calls the C function ts_headline_jsonb_byid_opt(), which calls
transform_jsonb_string_values(), which calls

it = JsonbIteratorInit(&jsonb->root);
is_scalar = it->isScalar;

but it->isScalar is not always initialized by JsonbIteratorInit(). (So
the 127 is quite likely clobbered memory.)

It can be fixed this way:

--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
JsonbIterator *parent)
{
JsonbIterator *it;

- it = palloc(sizeof(JsonbIterator));
+ it = palloc0(sizeof(JsonbIterator));
it->container = container;
it->parent = parent;
it->nElems = JsonContainerSize(container);

It's probably not a problem in practice, since the isScalar business is
apparently only used in the array case, but it's dubious to leave things
uninitialized like this nonetheless.

I've seen it earlier but couldn't decide what my proposed fix should
look like. One of the options I considered was:

--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void
*action_state,
JsonbIteratorToken type;
JsonbParseState *st = NULL;
text       *out;
-       bool            is_scalar = false;

it = JsonbIteratorInit(&jsonb->root);
- is_scalar = it->isScalar;

while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
{
@@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
*action_state,
}

if (res->type == jbvArray)
-               res->val.array.rawScalar = is_scalar;
+               res->val.array.rawScalar =
JsonContainerIsScalar(&jsonb->root);

return JsonbValueToJsonb(res);
}

palloc0 seems cleaner.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 26 May 2018 at 16:47, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
On 05/26/2018 03:09 AM, Piotr Stefaniak wrote:

On 2018-05-26 02:02, Peter Eisentraut wrote:

It can be fixed this way:

--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
JsonbIterator *parent)
{
JsonbIterator *it;

- it = palloc(sizeof(JsonbIterator));
+ it = palloc0(sizeof(JsonbIterator));
it->container = container;
it->parent = parent;
it->nElems = JsonContainerSize(container);

It's probably not a problem in practice, since the isScalar business is
apparently only used in the array case, but it's dubious to leave things
uninitialized like this nonetheless.

Yes, sounds reasonable.

I've seen it earlier but couldn't decide what my proposed fix should
look like. One of the options I considered was:

--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void
*action_state,
JsonbIteratorToken type;
JsonbParseState *st = NULL;
text       *out;
-       bool            is_scalar = false;

it = JsonbIteratorInit(&jsonb->root);
- is_scalar = it->isScalar;

while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
{
@@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
*action_state,
}

if (res->type == jbvArray)
-               res->val.array.rawScalar = is_scalar;
+               res->val.array.rawScalar =
JsonContainerIsScalar(&jsonb->root);

return JsonbValueToJsonb(res);
}

palloc0 seems cleaner.

Totally agree, palloc0 looks better (although I assume it's going to be
negligibly slower in those cases that aren't affected by this problem).