From e060cd59fabda2a0b2a7e119a58887791d030942 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 19 Mar 2023 15:36:16 -0400 Subject: [PATCH] Avoid copying undefined data in _readA_Const(). nodeRead() will have created a Node struct that's only allocated big enough for the specific node type, so copying sizeof(union ValUnion) can be copying too much. This provokes valgrind complaints, and with very bad luck could perhaps result in SIGSEGV. While at it, tidy up _equalA_Const to avoid duplicate checks of isnull. Per report from Alexander Lakhin. This code is new as of a6bc33019, so no need to back-patch. Discussion: https://postgr.es/m/4995256b-cc65-170e-0b22-60ad2cd535f1@gmail.com --- src/backend/nodes/equalfuncs.c | 8 +++----- src/backend/nodes/readfuncs.c | 25 ++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index b8a0452565..06d498f029 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -133,13 +133,11 @@ _equalExtensibleNode(const ExtensibleNode *a, const ExtensibleNode *b) static bool _equalA_Const(const A_Const *a, const A_Const *b) { - /* - * Hack for in-line val field. Also val is not valid is isnull is true. - */ - if (!a->isnull && !b->isnull && + COMPARE_SCALAR_FIELD(isnull); + /* Hack for in-line val field. Also val is not valid if isnull is true */ + if (!a->isnull && !equal(&a->val, &b->val)) return false; - COMPARE_SCALAR_FIELD(isnull); COMPARE_LOCATION_FIELD(location); return true; diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index f3629cdfd1..597e5b3ea8 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -305,6 +305,7 @@ _readA_Const(void) { READ_LOCALS(A_Const); + /* We expect either NULL or :val here */ token = pg_strtok(&length); if (length == 4 && strncmp(token, "NULL", 4) == 0) local_node->isnull = true; @@ -312,7 +313,29 @@ _readA_Const(void) { union ValUnion *tmp = nodeRead(NULL, 0); - memcpy(&local_node->val, tmp, sizeof(*tmp)); + /* To forestall valgrind complaints, copy only the valid data */ + switch (nodeTag(tmp)) + { + case T_Integer: + memcpy(&local_node->val, tmp, sizeof(Integer)); + break; + case T_Float: + memcpy(&local_node->val, tmp, sizeof(Float)); + break; + case T_Boolean: + memcpy(&local_node->val, tmp, sizeof(Boolean)); + break; + case T_String: + memcpy(&local_node->val, tmp, sizeof(String)); + break; + case T_BitString: + memcpy(&local_node->val, tmp, sizeof(BitString)); + break; + default: + elog(ERROR, "unrecognized node type: %d", + (int) nodeTag(tmp)); + break; + } } READ_LOCATION_FIELD(location);