Обсуждение: Unlikely-to-happen crash in ecpg driver caused by NULL-pointer check not done
Unlikely-to-happen crash in ecpg driver caused by NULL-pointer check not done
От
Michael Paquier
Дата:
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. This issue has been pointed out by Coverity, and I guessed the legwork needed by myself. Regards, -- Michael
Вложения
Re: Unlikely-to-happen crash in ecpg driver caused by NULL-pointer check not done
От
Heikki Linnakangas
Дата:
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
Re: Unlikely-to-happen crash in ecpg driver caused by NULL-pointer check not done
От
Michael Paquier
Дата:
On Tue, Feb 3, 2015 at 7:50 PM, Heikki Linnakangas <hlinnakangas@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
Вложения
Re: Unlikely-to-happen crash in ecpg driver caused by NULL-pointer check not done
От
Michael Meskes
Дата:
On Tue, Feb 03, 2015 at 10:44:17PM +0900, Michael Paquier wrote: > All those things are addressed in the patch attached. Fixed a typo and commited. Thanks Michael for fixing and Heikki for reviewing. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, 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