Fix issue 2093: pfree() called with a NULL pointer (#2095) (#2104)

Fixed issue 2093 where pfree() was called with a NULL pointer. The issue is due to some confusion with pfree(). There are 2 definitions for it, one that checks for a passed NULL and the other which does not. Created a function, pfree_if_not_null(), to check for NULL and call pfree() if a NULL wasn't passed. Modified the pfree references in the following files - src/backend/commands/label_commands.c src/backend/executor/cypher_merge.c src/backend/executor/cypher_set.c src/backend/executor/cypher_utils.c src/backend/parser/ag_scanner.l src/backend/parser/cypher_analyze.c src/backend/parser/cypher_expr.c src/backend/parser/cypher_gram.y src/backend/parser/cypher_parse_agg.c src/backend/utils/adt/age_global_graph.c src/backend/utils/adt/age_graphid_ds.c src/backend/utils/adt/age_session_info.c src/backend/utils/adt/age_vle.c src/backend/utils/adt/agtype.c src/backend/utils/adt/agtype_gin.c src/backend/utils/adt/agtype_raw.c src/backend/utils/adt/agtype_util.c src/backend/utils/load/ag_load_edges.c src/backend/utils/load/ag_load_labels.c src/include/utils/age_graphid_ds.h src/include/utils/age_session_info.h src/include/utils/agtype.h Added regression tests for the original issue. Resolved Conflicts: src/backend/commands/label_commands.c src/backend/parser/cypher_expr.c src/backend/parser/cypher_parse_agg.c src/backend/utils/adt/age_global_graph.c src/backend/utils/adt/agtype.c
apache · Sep 11, 2024 · 6eeceae · 6eeceae
1 parent e4f545a
commit 6eeceae
Show file tree

Hide file tree

Showing 23 changed files with 135 additions and 96 deletions.
diff --git a/regress/expected/expr.out b/regress/expected/expr.out
@@ -8506,6 +8506,21 @@ SELECT agtype_to_int8(bool('neither'));
 ERROR:  invalid input syntax for type boolean: "neither"
 LINE 1: SELECT agtype_to_int8(bool('neither'));
                                    ^
+--
+-- Issue 2093: Server crashes when executing SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+--
+SELECT agtype_access_operator(agtype_in('[null, null]'));
+ agtype_access_operator 
+------------------------
+
+(1 row)
+
+SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+ agtype_hash_cmp 
+-----------------
+      -505290721
+(1 row)
+
 --
 -- Cleanup
 --

diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql
@@ -3455,6 +3455,12 @@ SELECT agtype_to_int2(bool('neither'));
 SELECT agtype_to_int4(bool('neither'));
 SELECT agtype_to_int8(bool('neither'));
 
+--
+-- Issue 2093: Server crashes when executing SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+--
+SELECT agtype_access_operator(agtype_in('[null, null]'));
+SELECT agtype_hash_cmp(agtype_in('[null, null, null, null, null]'));
+
 --
 -- Cleanup
 --

diff --git a/src/backend/commands/label_commands.c b/src/backend/commands/label_commands.c
@@ -94,8 +94,6 @@ static void range_var_callback_for_remove_relation(const RangeVar *rel,
                                                    Oid odl_rel_oid,
                                                    void *arg);
 
-
-
 PG_FUNCTION_INFO_V1(create_vlabel);
 
 /*

diff --git a/src/backend/executor/cypher_merge.c b/src/backend/executor/cypher_merge.c
@@ -346,7 +346,7 @@ static void free_path_entry_array(path_entry **path_array, int length)
 
     for (index = 0; index < length; index++)
     {
-        pfree(path_array[index]);
+        pfree_if_not_null(path_array[index]);
     }
 }
 
@@ -892,10 +892,10 @@ static void end_cypher_merge(CustomScanState *node)
         free_path_entry_array(entry, path_length);
 
         /* free up the array container */
-        pfree(entry);
+        pfree_if_not_null(entry);
 
         /* free up the created_path container */
-        pfree(css->created_paths_list);
+        pfree_if_not_null(css->created_paths_list);
 
         css->created_paths_list = next;
     }

diff --git a/src/backend/executor/cypher_set.c b/src/backend/executor/cypher_set.c
@@ -594,7 +594,7 @@ static void process_update_list(CustomScanState *node)
         lidx++;
     }
     /* free our lookup array */
-    pfree(luindex);
+    pfree_if_not_null(luindex);
 }
 
 static TupleTableSlot *exec_cypher_set(CustomScanState *node)

diff --git a/src/backend/executor/cypher_utils.c b/src/backend/executor/cypher_utils.c
@@ -75,7 +75,7 @@ uint32 datum_image_hash(Datum value, bool typByVal, int typLen)
         /* Only free memory if it's a copy made here. */
         if ((Pointer) val != (Pointer) value)
         {
-            pfree(val);
+            pfree_if_not_null(val);
         }
     }
     else if (typLen == -2)

diff --git a/src/backend/parser/ag_scanner.l b/src/backend/parser/ag_scanner.l
@@ -34,6 +34,7 @@
 #include "mb/pg_wchar.h"
 
 #include "parser/ag_scanner.h"
+#include "utils/agtype.h"
 }
 
 %option 8bit
@@ -796,7 +797,7 @@ void *ag_yyrealloc(void *ptr, yy_size_t size, yyscan_t yyscanner)
     {
         if (size == 0)
         {
-            pfree(ptr);
+            pfree_if_not_null(ptr);
             return NULL;
         }
         else
@@ -813,7 +814,7 @@ void *ag_yyrealloc(void *ptr, yy_size_t size, yyscan_t yyscanner)
 void ag_yyfree(void *ptr, yyscan_t yyscanner)
 {
     if (ptr)
-        pfree(ptr);
+        pfree_if_not_null(ptr);
 }
 
 static void strbuf_init(strbuf *sb, int capacity)
@@ -826,7 +827,7 @@ static void strbuf_init(strbuf *sb, int capacity)
 static void strbuf_cleanup(strbuf *sb)
 {
     if (sb->buffer)
-        pfree(sb->buffer);
+        pfree_if_not_null(sb->buffer);
 }
 
 static void strbuf_append_buf(strbuf *sb, const char *b, const int len)
@@ -1121,8 +1122,8 @@ static void _numstr_to_decimal(const char *numstr, const int base, strbuf *sb)
         strbuf_append_buf(sb, &buf[buf_i], NDIGITS_PER_REMAINDER - buf_i);
     }
 
-    pfree(remainders);
-    pfree(words);
+    pfree_if_not_null(remainders);
+    pfree_if_not_null(words);
 }
 
 static uint32 hexdigit_value(const char c)

diff --git a/src/backend/parser/cypher_analyze.c b/src/backend/parser/cypher_analyze.c
@@ -106,7 +106,7 @@ static void post_parse_analyze(ParseState *pstate, Query *query)
         }
 
         /* reset extra_node */
-        pfree(extra_node);
+        pfree_if_not_null(extra_node);
         extra_node = NULL;
     }
 }
@@ -275,7 +275,7 @@ static void build_explain_query(Query *query, Node *explain_node)
     ((ExplainStmt *)explain_node)->options = NULL;
 
     /* we need to free query_node as it is no longer needed */
-    pfree(query_node);
+    pfree_if_not_null(query_node);
 }
 
 static bool is_rte_cypher(RangeTblEntry *rte)

diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y
@@ -26,6 +26,7 @@
 #include "parser/cypher_gram.h"
 #include "parser/cypher_parse_node.h"
 #include "parser/scansup.h"
+#include "utils/agtype.h"
 
 /* override the default action for locations */
 #define YYLLOC_DEFAULT(current, rhs, n) \
@@ -2796,7 +2797,7 @@ static char *create_unique_name(char *prefix_name)
     /* if we created the prefix, we need to free it */
     if (prefix_name == NULL || strlen(prefix_name) <= 0)
     {
-        pfree(prefix);
+        pfree_if_not_null(prefix);
     }
 
     return name;

diff --git a/src/backend/parser/cypher_parse_agg.c b/src/backend/parser/cypher_parse_agg.c
@@ -31,6 +31,7 @@
 #include "parser/cypher_parse_agg.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteManip.h"
+#include "utils/agtype.h"
 
 typedef struct
 {
@@ -846,7 +847,7 @@ static List * expand_grouping_sets(List *groupingSets, int limit)
         while (result_len-- > 0)
             result = lappend(result, *ptr++);
 
-        pfree(buf);
+        pfree_if_not_null(buf);
     }
 
     return result;

diff --git a/src/backend/utils/adt/age_global_graph.c b/src/backend/utils/adt/age_global_graph.c
@@ -182,7 +182,7 @@ static void create_GRAPH_global_hashtables(GRAPH_global_context *ggctx)
     ggctx->vertex_hashtable = hash_create(vhn, VERTEX_HTAB_INITIAL_SIZE,
                                           &vertex_ctl,
                                           HASH_ELEM | HASH_FUNCTION);
-    pfree(vhn);
+    pfree_if_not_null(vhn);
 
     /* initialize the edge hashtable */
     MemSet(&edge_ctl, 0, sizeof(edge_ctl));
@@ -191,7 +191,7 @@ static void create_GRAPH_global_hashtables(GRAPH_global_context *ggctx)
     edge_ctl.hash = tag_hash;
     ggctx->edge_hashtable = hash_create(ehn, EDGE_HTAB_INITIAL_SIZE, &edge_ctl,
                                         HASH_ELEM | HASH_FUNCTION);
-    pfree(ehn);
+    pfree_if_not_null(ehn);
 }
 
 /* helper function to get a List of all label names for the specified graph */
@@ -625,7 +625,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
     }
 
     /* free the graph name */
-    pfree(ggctx->graph_name);
+    pfree_if_not_null(ggctx->graph_name);
     ggctx->graph_name = NULL;
 
     ggctx->graph_oid = InvalidOid;
@@ -657,7 +657,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
         }
 
         /* free the vertex's datumCopy properties */
-        pfree(DatumGetPointer(value->vertex_properties));
+        pfree_if_not_null(DatumGetPointer(value->vertex_properties));
         value->vertex_properties = 0;
 
         /* free the edge list associated with this vertex */
@@ -699,7 +699,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
         }
 
         /* free the edge's datumCopy properties */
-        pfree(DatumGetPointer(value->edge_properties));
+        pfree_if_not_null(DatumGetPointer(value->edge_properties));
         value->edge_properties = 0;
 
         /* move to the next edge */
@@ -722,7 +722,7 @@ static bool free_specific_GRAPH_global_context(GRAPH_global_context *ggctx)
     ggctx->edge_hashtable = NULL;
 
     /* free the context */
-    pfree(ggctx);
+    pfree_if_not_null(ggctx);
     ggctx = NULL;
 
     return true;
@@ -1225,7 +1225,7 @@ Datum age_vertex_stats(PG_FUNCTION_ARGS)
     ggctx = manage_GRAPH_global_contexts(graph_name, graph_oid);
 
     /* free the graph name */
-    pfree(graph_name);
+    pfree_if_not_null(graph_name);
 
     /* get the id */
     agtv_temp = GET_AGTYPE_VALUE_OBJECT_VALUE(agtv_vertex, "id");

diff --git a/src/backend/utils/adt/age_graphid_ds.c b/src/backend/utils/adt/age_graphid_ds.c
@@ -144,14 +144,14 @@ void free_ListGraphId(ListGraphId *container)
     {
         next_node = curr_node->next;
         /* we can do this because this is just a list of ints */
-        pfree(curr_node);
+        pfree_if_not_null(curr_node);
         container->size--;
         curr_node = next_node;
     }
 
     Assert(container->size == 0);
     /* free the container */
-    pfree(container);
+    pfree_if_not_null(container);
 }
 
 /* helper function to create a new, empty, graphid stack */
@@ -188,7 +188,7 @@ void free_graphid_stack(ListGraphId *stack)
         GraphIdNode *next = stack->head->next;
 
         /* free the head element */
-        pfree(stack->head);
+        pfree_if_not_null(stack->head);
         /* move the head to the next */
         stack->head = next;
     }
@@ -253,7 +253,7 @@ graphid pop_graphid_stack(ListGraphId *stack)
     stack->head = stack->head->next;
     stack->size--;
     /* free the element */
-    pfree(node);
+    pfree_if_not_null(node);
 
     /* return the id */
     return id;

diff --git a/src/backend/utils/adt/age_session_info.c b/src/backend/utils/adt/age_session_info.c
@@ -125,12 +125,12 @@ void reset_session_info(void)
     {
         if (session_info_graph_name != NULL)
         {
-            pfree(session_info_graph_name);
+            pfree_if_not_null(session_info_graph_name);
         }
 
         if (session_info_cypher_statement != NULL)
         {
-            pfree(session_info_cypher_statement);
+            pfree_if_not_null(session_info_cypher_statement);
         }
     }
 

diff --git a/src/backend/utils/adt/age_vle.c b/src/backend/utils/adt/age_vle.c
@@ -313,7 +313,7 @@ static void create_VLE_local_state_hashtable(VLE_local_context *vlelctx)
                                                 EDGE_STATE_HTAB_INITIAL_SIZE,
                                                 &edge_state_ctl,
                                                 HASH_ELEM | HASH_FUNCTION);
-    pfree(eshn);
+    pfree_if_not_null(eshn);
 }
 
 /*
@@ -400,14 +400,14 @@ static void free_VLE_local_context(VLE_local_context *vlelctx)
     /* free the stored graph name */
     if (vlelctx->graph_name != NULL)
     {
-        pfree(vlelctx->graph_name);
+        pfree_if_not_null(vlelctx->graph_name);
         vlelctx->graph_name = NULL;
     }
 
     /* free the stored edge label name */
     if (vlelctx->edge_label_name != NULL)
     {
-        pfree(vlelctx->edge_label_name);
+        pfree_if_not_null(vlelctx->edge_label_name);
         vlelctx->edge_label_name = NULL;
     }
 
@@ -429,15 +429,15 @@ static void free_VLE_local_context(VLE_local_context *vlelctx)
     }
 
     /* free the containers */
-    pfree(vlelctx->dfs_vertex_stack);
-    pfree(vlelctx->dfs_edge_stack);
-    pfree(vlelctx->dfs_path_stack);
+    pfree_if_not_null(vlelctx->dfs_vertex_stack);
+    pfree_if_not_null(vlelctx->dfs_edge_stack);
+    pfree_if_not_null(vlelctx->dfs_path_stack);
     vlelctx->dfs_vertex_stack = NULL;
     vlelctx->dfs_edge_stack = NULL;
     vlelctx->dfs_path_stack = NULL;
 
     /* and finally the context itself */
-    pfree(vlelctx);
+    pfree_if_not_null(vlelctx);
     vlelctx = NULL;
 }