Lack of Sanity Checking in file 'misc.c' for PostgreSQL 9.4.x
============================================================================
POSTGRESQL BUG REPORT TEMPLATE
============================================================================
Your name : Bill Parker
Your email address : wp02855 at gmail dot com
System Configuration:
---------------------
Architecture (example: Intel Pentium) : x86/x86-64/AMD
Operating System (example: Linux 2.4.18) : Linux 3.11.6-4
PostgreSQL version (example: PostgreSQL 9.4.3): PostgreSQL 9.4.x
Compiler used (example: gcc 3.3.5) : gcc version 4.8.1
Please enter a FULL description of your problem:
------------------------------------------------
Hello All,
In reviewing some code, in directory
'postgresql-9.4.3/src/interfaces/ecpg/ecpglib',
file 'misc.c', there are several instances where a call to malloc()
is made, but no check for a return value of NULL is made, which
would indicate failure. Additionally, if sqlca = malloc() fails,
ecpg_init_sqlca would be called with variable 'sqlca' equal to NULL?
If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------
The patch file below addresses these issues:
--- misc.c.orig 2015-06-11 09:23:13.807020490 -0700
+++ misc.c 2015-06-11 09:32:10.077177669 -0700
@@ -143,6 +143,9 @@
if (sqlca == NULL)
{
sqlca = malloc(sizeof(struct sqlca_t));
+ if (sqlca == NULL) { /* malloc() failed, now what
should we do? */
+ ecpg_log("Unable to allocate memory in
ECPGget_sqlca()\n");
+ }
ecpg_init_sqlca(sqlca);
pthread_setspecific(sqlca_key, sqlca);
}Please feel free to review and comment on the above patch file...
I am attaching the patch file to this bug report
Bill Parker (wp02855 at gmail dot com)
Attachments:
misc.c.patchapplication/octet-stream; name=misc.c.patchDownload+3-0
(Adding Michael Meskes in CC:)
On Fri, Jun 12, 2015 at 4:11 AM, Bill Parker <wp02855@gmail.com> wrote:
--- misc.c.orig 2015-06-11 09:23:13.807020490 -0700 +++ misc.c 2015-06-11 09:32:10.077177669 -0700 @@ -143,6 +143,9 @@ if (sqlca == NULL) { sqlca = malloc(sizeof(struct sqlca_t)); + if (sqlca == NULL) { /* malloc() failed, now what should we do? */ + ecpg_log("Unable to allocate memory in ECPGget_sqlca()\n"); + } ecpg_init_sqlca(sqlca); pthread_setspecific(sqlca_key, sqlca); }Please feel free to review and comment on the above patch file...
I am attaching the patch file to this bug report
Bill Parker (wp02855 at gmail dot com)
Nice catch. Regarding your patch, it seems to me that this is not
enough. I think that we should return NULL with ECPGget_sqlca in case
of an OOM instead of logging a message with ecpg_log and let each code
path decide what to do when sqlca is NULL. Some code paths can
directly use ecpg_raise with ECPG_OUT_OF_MEMORY. Other code paths,
like the ones in error.c will need to show up with appropriate error
messages.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Thu, Jun 11, 2015 at 12:11:37PM -0700, Bill Parker wrote:
file 'misc.c', there are several instances where a call to malloc()
is made, but no check for a return value of NULL is made, which
Your patch lists one such case, but where are the other "several"?
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Jun 12, 2015 at 03:53:43PM +0900, Michael Paquier wrote:
(Adding Michael Meskes in CC:)
Thanks.
Nice catch. Regarding your patch, it seems to me that this is not
enough. I think that we should return NULL with ECPGget_sqlca in case
of an OOM instead of logging a message with ecpg_log and let each code
path decide what to do when sqlca is NULL. Some code paths can
directly use ecpg_raise with ECPG_OUT_OF_MEMORY. Other code paths,
like the ones in error.c will need to show up with appropriate error
messages.
Agreed on all accounts.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Fri, Jun 12, 2015 at 10:06 PM, Michael Meskes <meskes@postgresql.org> wrote:
On Fri, Jun 12, 2015 at 03:53:43PM +0900, Michael Paquier wrote:
(Adding Michael Meskes in CC:)
Thanks.
Nice catch. Regarding your patch, it seems to me that this is not
enough. I think that we should return NULL with ECPGget_sqlca in case
of an OOM instead of logging a message with ecpg_log and let each code
path decide what to do when sqlca is NULL. Some code paths can
directly use ecpg_raise with ECPG_OUT_OF_MEMORY. Other code paths,
like the ones in error.c will need to show up with appropriate error
messages.Agreed on all accounts.
So, here is a patch implementing those ideas. In code paths where a
line number is available ecpg_raise() is called to report the error.
In other paths ecpg_log() is used to log an "out of memory" message.
Now, the routines of error.c, like ecpg_raise() can fail as well their
malloc() call, hence it seems adapted to me to fallback to ecpg_log()
and report the error to the user.
Thoughts?
--
Michael
Attachments:
20150615_ecpg_fix_malloc.patchbinary/octet-stream; name=20150615_ecpg_fix_malloc.patchDownload+104-1
On Mon, Jun 15, 2015 at 04:35:47PM +0900, Michael Paquier wrote:
So, here is a patch implementing those ideas. In code paths where a
line number is available ecpg_raise() is called to report the error.
In other paths ecpg_log() is used to log an "out of memory" message.
Now, the routines of error.c, like ecpg_raise() can fail as well their
malloc() call, hence it seems adapted to me to fallback to ecpg_log()
and report the error to the user.
Thoughts?
Sounds good to me. Patch committed.
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
On Mon, Jun 15, 2015 at 9:34 PM, Michael Meskes <meskes@postgresql.org> wrote:
On Mon, Jun 15, 2015 at 04:35:47PM +0900, Michael Paquier wrote:
So, here is a patch implementing those ideas. In code paths where a
line number is available ecpg_raise() is called to report the error.
In other paths ecpg_log() is used to log an "out of memory" message.
Now, the routines of error.c, like ecpg_raise() can fail as well their
malloc() call, hence it seems adapted to me to fallback to ecpg_log()
and report the error to the user.
Thoughts?Sounds good to me. Patch committed.
Thanks.
--
Michael
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs