Re: [PATCH] dynahash: add memory allocation failure check - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH] dynahash: add memory allocation failure check
Date
Msg-id 1484393.1745437373@sss.pgh.pa.us
Whole thread Raw
In response to [PATCH] dynahash: add memory allocation failure check  (m.korotkov@postgrespro.ru)
Responses Re: [PATCH] dynahash: add memory allocation failure check
List pgsql-hackers
m.korotkov@postgrespro.ru writes:
> I found a case of potential NULL pointer dereference.
> In src/backend/utils/hash/dynahash.c in function HTAB *hash_create() the 
> result of the DynaHashAlloc() is used unsafely.
> The function DynaHashAlloc() calls MemoryContextAllocExtended() with 
> MCXT_ALLOC_NO_OOM and can return a NULL pointer.

Ugh, that's a stupid bug.  Evidently my fault, too (9c911ec06).

> Added the pointer check for avoiding a potential problem.

This doesn't seem like a nice way to fix it.  The code right there is
assuming palloc semantics, which was okay before 9c911ec06, but is so
no longer.  I think the right thing is to put it back to palloc
semantics, which means not using DynaHashAlloc there, as attached.

            regards, tom lane

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 3f25929f2d8..1ad155d446e 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -390,7 +390,8 @@ hash_create(const char *tabname, long nelem, const HASHCTL *info, int flags)
     }

     /* Initialize the hash header, plus a copy of the table name */
-    hashp = (HTAB *) DynaHashAlloc(sizeof(HTAB) + strlen(tabname) + 1);
+    hashp = (HTAB *) MemoryContextAlloc(CurrentDynaHashCxt,
+                                        sizeof(HTAB) + strlen(tabname) + 1);
     MemSet(hashp, 0, sizeof(HTAB));

     hashp->tabname = (char *) (hashp + 1);

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgsql: Add function to get memory context stats for processes
Next
From: Andrew Dunstan
Date:
Subject: Re: Cygwin support