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