Thread: NULL dereference when memory is tight
Hi,
I recently found a few places in the latest beta release where a NULL
dereference could occur when insufficient memory is available.
For example in connection_type.c:
830 self->dsn = strdup(dsn);
...
855 pos = strstr(self->dsn, "password");
strdup could return a NULL.
Admittedly this is probably a minor bug, but would it interest anyone if
I report these somewhere?
--
Brian Sutherland
On Sun, Feb 20, 2011 at 4:47 PM, Brian Sutherland <brian@vanguardistas.net> wrote: > Hi, > > I recently found a few places in the latest beta release where a NULL > dereference could occur when insufficient memory is available. > > For example in connection_type.c: > > 830 self->dsn = strdup(dsn); > ... > 855 pos = strstr(self->dsn, "password"); > > strdup could return a NULL. > > Admittedly this is probably a minor bug, but would it interest anyone if > I report these somewhere? Thank you for the review. Having patches would be even better, but I will take care of this one. Cheers -- Daniele
On Sun, Feb 20, 2011 at 06:06:30PM +0000, Daniele Varrazzo wrote: > On Sun, Feb 20, 2011 at 4:47 PM, Brian Sutherland > <brian@vanguardistas.net> wrote: > > Hi, > > > > I recently found a few places in the latest beta release where a NULL > > dereference could occur when insufficient memory is available. > > > > For example in connection_type.c: > > > > 830 self->dsn = strdup(dsn); > > ... > > 855 pos = strstr(self->dsn, "password"); > > > > strdup could return a NULL. > > > > Admittedly this is probably a minor bug, but would it interest anyone if > > I report these somewhere? > > Thank you for the review. Actually, thanks to monoidics for letting me try out their INFER static code checker. > Having patches would be even better, but I > will take care of this one. Great! Attached is a patch for another issue, though I'm not sure if calling PyErr_NoMemory within libpq is sane. To tell if the other issues INFER raises are bugs would require a much deeper insight into the psycopg2 code than I have. -- Brian Sutherland
Attachment
On 24/02/11 10:39, Brian Sutherland wrote: > Attached is a patch for another issue, though I'm not sure if calling > PyErr_NoMemory within libpq is sane. I won't call any Py* function without holding the GIL. That's why conn_notice_callback() uses its own data structure instead of a simple Python list. federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it The reverse side also has a reverse side. -- Japanese proverb
On Thu, Feb 24, 2011 at 9:52 AM, Federico Di Gregorio <federico.digregorio@dndg.it> wrote: > On 24/02/11 10:39, Brian Sutherland wrote: >> Attached is a patch for another issue, though I'm not sure if calling >> PyErr_NoMemory within libpq is sane. > > I won't call any Py* function without holding the GIL. That's why > conn_notice_callback() uses its own data structure instead of a simple > Python list. I've fixed the potential access to uninitialized memory by simply discarding the notice in case allocation fails. We could additionally print the notice on stderr but I think the process is doomed anyway if it just failed to allocate a struct of two pointers. Brian, if you want to keep on playing with INFER, you should probably pull the outstanding patches from my devel branch <https://github.com/dvarrazzo/psycopg/> as in the last days I've cleaned up several allocation glitches throughout all the library. Thank you, cheers! -- Danele
On 24/02/11 11:32, Daniele Varrazzo wrote: > On Thu, Feb 24, 2011 at 9:52 AM, Federico Di Gregorio > <federico.digregorio@dndg.it> wrote: >> On 24/02/11 10:39, Brian Sutherland wrote: >>> Attached is a patch for another issue, though I'm not sure if calling >>> PyErr_NoMemory within libpq is sane. >> >> I won't call any Py* function without holding the GIL. That's why >> conn_notice_callback() uses its own data structure instead of a simple >> Python list. > > I've fixed the potential access to uninitialized memory by simply > discarding the notice in case allocation fails. We could additionally > print the notice on stderr but I think the process is doomed anyway if > it just failed to allocate a struct of two pointers. > > Brian, if you want to keep on playing with INFER, you should probably > pull the outstanding patches from my devel branch > <https://github.com/dvarrazzo/psycopg/> as in the last days I've > cleaned up several allocation glitches throughout all the library. All reviewd, merged and pushed. federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it Se sai che hai un ***** di file così, lo manovri subito. -- vodka
On Thu, Feb 24, 2011 at 10:39 AM, Federico Di Gregorio <federico.digregorio@dndg.it> wrote: > All reviewd, merged and pushed. Great! FYI, Jason has a few patches in his github repos dealing with warnings and other cleanups for windows. One (7f26fdb2) is probably better fixed another way. Once he says he is ok, I'd say we finished with this release. -- Daniele