On 02/03/2015 09:28 AM, Michael Paquier wrote:
Hi all,

In ecpg_add_mem of memory.c, we use ecpg_alloc but there is actually
no NULL-pointer check. If an OOM shows up exactly at this point, this
is likely to cause a crash. Attached patch adds some extra processing
to ecpg_add_mem to check if the allocation fails, and to fail properly
if an OOM appears.

--- a/src/interfaces/ecpg/ecpglib/descriptor.c
+++ b/src/interfaces/ecpg/ecpglib/descriptor.c
@@ -440,7 +440,12 @@ ECPGget_desc(int lineno, const char *desc_name, int 
index,...)
                                                return false;
                                        }
                                        *(void **) var = mem;
-                                       ecpg_add_mem(mem, lineno);
+                                       if (!ecpg_add_mem(mem, lineno))
+                                       {
+                                               ecpg_free(mem);
+                                               va_end(args);
+                                               return false;
+                                       }
                                        var = mem;
                                }

Hmm. Since the ecpg_add_mem call is done after setting (*(void **) var), that's left to point to already-free'd memory. The other call sites have a similar issue. I haven't analyzed the code to check if that's harmless or not, but seems better to not do that.

In ecpg_add_mem, the ecpg_raise() call is unnecessary, since ecpg_alloc already does that on failure.

(It would be less error-prone to have an ecpg_alloc_auto() function that combines ecpg_alloc+ecpg_add_mem in one call.)

/* Here are some methods used by the lib. */

/* Returns a pointer to a string containing a simple type name. */
bool            ecpg_add_mem(void *ptr, int lineno);

bool ecpg_get_data(const PGresult *, int, int, int, enum ECPGttype type,
                          enum ECPGttype, char *, char *, long, long, long,
                          enum ARRAY_TYPE, enum COMPAT_MODE, bool);

That second comment is completely bogus. It's not this patch's fault, it's been like that forever, but just happened to notice..

- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to