scipy: BUG: malloc() calls in Cython and C that are not checked for failure.

The C function malloc() returns NULL (i.e. 0) if it fails to allocate memory. The return value of a malloc() call must be checked for this value before attempting to use the result. Here are places in the scipy code where this check is not made. If any of these calls fails, the result is likely to be a segmentation fault.

Cython

  • cluster In the file _optimal_leaf_ordering.pyx, in the function identify_swaps
  • interpolate In the file _ppoly.pyx, in the functions real_roots, croots_poly1 and _croots_poly1
  • spatial In setlist.pxd, in the function init, in the statement setlists.sets[j] = ...
  • sparse.csgraph File _shortest_path.pyx, in the code that creates nodes in the functions _dijkstra_directed, _dijkstra_directed_multi, _dijkstra_undirected, _dijkstra_undirected_multi
  • _lib In the file _ccallback_c.pyx, the result of strdup is used without checking for NULL. strdup can fail to allocate memory for the duplicated string. In that case, it returns NULL. (Note: this might be harmless. I haven’t checked to see if the pointer is ever dereferenced while the capsule is in use.)

C

  • fftpack In the file fftpack.h, the macro GEN_CACHE is defined. It is used in the files convolve.c, drfft.c, zfft.c,zfftnd.c
  • ndimage In the file src/ni_morphology.c, at line 624 (at the time of this writing): temp->coordinates = malloc(...)
  • signal File sigtoolsmodule.c, lines 914–927 (8 unchecked calls to malloc) and line 1096

We have vendored code in sparse and spatial that use malloc:

  • The SuperLU code in linalg/dsolve/SuperLU/SRC has its own memory management code. I didn’t spend time trying to figure out the memory management here. The file cmemory.c is the most relevant, and there appear to be calls to intMalloc() and user_malloc() that do not check for failure, but I didn’t dig deeper to see if those functions check for failure before returning.
  • spatial provides a wrapper for QHull. I didn’t look very deeply into how the qhull code manages memory. It looks like the function qh_memalloc() in mem_r.c will print an error and exit if it fails.

We should be aware of these vendored packages using malloc, but I think a thorough review of their memory management is not required. Someone can do a deep dive into either of these libraries in the future if there is ever a suspicion that they are leaking memory.

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 15 (14 by maintainers)

Most upvoted comments

FYI, I had this on a list of issues to finish off. I think it’s still useful to do, especially SuperLU - and then upstream and needed changes. Fine to leave the issue closed, but it may get done anyway.

@rgommers I’m happy to take a look at #10035 this weekend. Seems like the sort of issue I can make headway on.

@clementkng Maybe it would be good to postpone your work on fftpack. With a bit of luck, most of that code could be removed soon.