On Tue, Feb 3, 2015 at 7:50 PM, Heikki Linnakangas
<hlinnakan...@vmware.com> wrote:
> 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.
Right, it is an error do allocate this memory if there is a risk that
it may be freed. Hence fixed.
> In ecpg_add_mem, the ecpg_raise() call is unnecessary, since ecpg_alloc
> already does that on failure.
Right, check.
> (It would be less error-prone to have an ecpg_alloc_auto() function that
> combines ecpg_alloc+ecpg_add_mem in one call.)
It makes sense.
>> /* Here are some methods used by the lib. */
>>
>> /* Returns a pointer to a string containing a simple type name. */
> That second comment is completely bogus. It's not this patch's fault, it's
> been like that forever, but just happened to notice..
Corrected.
All those things are addressed in the patch attached.
--
Michael
From 206dc0dd95a83e056968ac822f7b9331ffcf82cd Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Tue, 3 Feb 2015 16:21:50 +0900
Subject: [PATCH] Fix unlikely-to-happen crash in ecpg_add_mem
This routine was called ecpg_alloc to allocate to memory but did not
actually check the returned pointer allocated, potentially NULL which
is actually the result of a malloc call.
Issue noted by Coverity, though I guessed the legwork needed here.
---
src/interfaces/ecpg/ecpglib/descriptor.c | 6 ++----
src/interfaces/ecpg/ecpglib/execute.c | 8 +++-----
src/interfaces/ecpg/ecpglib/extern.h | 4 ++--
src/interfaces/ecpg/ecpglib/memory.c | 22 +++++++++++++++++++++-
4 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c
index b2990ca..956c035 100644
--- a/src/interfaces/ecpg/ecpglib/descriptor.c
+++ b/src/interfaces/ecpg/ecpglib/descriptor.c
@@ -432,7 +432,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
/* allocate storage if needed */
if (arrsize == 0 && *(void **) var == NULL)
{
- void *mem = (void *) ecpg_alloc(offset * ntuples, lineno);
+ void *mem = (void *) ecpg_auto_alloc(offset * ntuples, lineno);
if (!mem)
{
@@ -440,7 +440,6 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
return false;
}
*(void **) var = mem;
- ecpg_add_mem(mem, lineno);
var = mem;
}
@@ -510,7 +509,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
/* allocate storage if needed */
if (data_var.ind_arrsize == 0 && data_var.ind_value == NULL)
{
- void *mem = (void *) ecpg_alloc(data_var.ind_offset * ntuples, lineno);
+ void *mem = (void *) ecpg_auto_alloc(data_var.ind_offset * ntuples, lineno);
if (!mem)
{
@@ -518,7 +517,6 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
return false;
}
*(void **) data_var.ind_pointer = mem;
- ecpg_add_mem(mem, lineno);
data_var.ind_value = mem;
}
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index 8a3dd75..46d884f 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -398,11 +398,10 @@ ecpg_store_result(const PGresult *results, int act_field,
}
ecpg_log("ecpg_store_result on line %d: allocating memory for %d tuples\n", stmt->lineno, ntuples);
- var->value = (char *) ecpg_alloc(len, stmt->lineno);
+ var->value = (char *) ecpg_auto_alloc(len, stmt->lineno);
if (!var->value)
return false;
*((char **) var->pointer) = var->value;
- ecpg_add_mem(var->value, stmt->lineno);
}
/* allocate indicator variable if needed */
@@ -410,11 +409,10 @@ ecpg_store_result(const PGresult *results, int act_field,
{
int len = var->ind_offset * ntuples;
- var->ind_value = (char *) ecpg_alloc(len, stmt->lineno);
- if (!var->ind_value)
+ var->ind_value = (char *) ecpg_auto_alloc(len, stmt->lineno);
+ if (!var_ind_value)
return false;
*((char **) var->ind_pointer) = var->ind_value;
- ecpg_add_mem(var->ind_value, stmt->lineno);
}
/* fill the variable with the tuple(s) */
diff --git a/src/interfaces/ecpg/ecpglib/extern.h b/src/interfaces/ecpg/ecpglib/extern.h
index 3836007..2b670e0 100644
--- a/src/interfaces/ecpg/ecpglib/extern.h
+++ b/src/interfaces/ecpg/ecpglib/extern.h
@@ -136,8 +136,7 @@ extern struct var_list *ivlist;
/* Here are some methods used by the lib. */
-/* Returns a pointer to a string containing a simple type name. */
-void ecpg_add_mem(void *ptr, int lineno);
+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,
@@ -148,6 +147,7 @@ void ecpg_pthreads_init(void);
#endif
struct connection *ecpg_get_connection(const char *);
char *ecpg_alloc(long, int);
+char *ecpg_auto_alloc(long, int);
char *ecpg_realloc(void *, long, int);
void ecpg_free(void *);
bool ecpg_init(const struct connection *, const char *, const int);
diff --git a/src/interfaces/ecpg/ecpglib/memory.c b/src/interfaces/ecpg/ecpglib/memory.c
index a09cd26..dffc3a7 100644
--- a/src/interfaces/ecpg/ecpglib/memory.c
+++ b/src/interfaces/ecpg/ecpglib/memory.c
@@ -104,14 +104,34 @@ static struct auto_mem *auto_allocs = NULL;
#define set_auto_allocs(am) do { auto_allocs = (am); } while(0)
#endif
-void
+char *
+ecpg_auto_alloc(long size, int lineno)
+{
+ void *ptr = (void *) ecpg_alloc(size, lineno);
+
+ if (!ptr)
+ return NULL;
+
+ if (!ecpg_add_mem(ptr, lineno))
+ {
+ ecpg_free(ptr);
+ return NULL;
+ }
+ return ptr;
+}
+
+bool
ecpg_add_mem(void *ptr, int lineno)
{
struct auto_mem *am = (struct auto_mem *) ecpg_alloc(sizeof(struct auto_mem), lineno);
+ if (!am)
+ return false;
+
am->pointer = ptr;
am->next = get_auto_allocs();
set_auto_allocs(am);
+ return true;
}
void
--
2.2.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
