classification
Title: Missing sanity checks for various C library function calls...
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, dogbert2, ned.deily, terry.reedy, vstinner
Priority: normal Keywords: patch

Created on 2015-04-06 17:00 by dogbert2, last changed 2019-08-28 22:02 by ZackerySpytz. This issue is now closed.

Files
File name Uploaded Description Edit
getpath.c.patch dogbert2, 2015-04-06 17:00
ffi.c.patch dogbert2, 2015-04-06 17:01 patch file (diff -u) for this bug
Pull Requests
URL Status Linked Edit
PR 15424 merged ZackerySpytz, 2019-08-23 15:29
PR 15555 open ZackerySpytz, 2019-08-28 22:02
Messages (8)
msg240160 - (view) Author: Bill Parker (dogbert2) * Date: 2015-04-06 17:00
Hello All,

   In reviewing code for Python-3.4.3 in directory
'Modules/_ctypes/libffi/src/arm', file 'ffi.c', I found a pair
of calls to calloc() which do not test for a return value
of NULL, indicating failure.  The patch file below corrects
this issue:

--- ffi.c.orig  2015-04-04 15:43:19.662709073 -0700
+++ ffi.c       2015-04-04 15:51:27.142665269 -0700
@@ -629,12 +629,21 @@
 
     /* We have valid trampoline and config pages */
     table = calloc (1, sizeof(ffi_trampoline_table));
+    if (table == NULL) { /* oops, calloc() failed, now what??? */
+      fprintf(stderr, "vm calloc() failure: %d at %s:%d\n", kt, __FILE__, __LINE__);
+      return NULL; /* go home??? */
+    }
     table->free_count = FFI_TRAMPOLINE_COUNT;
     table->config_page = config_page;
     table->trampoline_page = trampoline_page;
 
     /* Create and initialize the free list */
     table->free_list_pool = calloc(FFI_TRAMPOLINE_COUNT, sizeof(ffi_trampoline_table_entry));
+    if (table->free_list_pool == NULL) { /* oops, calloc() failed, now what */
+      fprintf(stderr, "vm calloc() failure: %d at %s:%d\n", kt, __FILE__, __LINE__);
+      free(table);  /* free table (from previos calloc() call) */
+      return NULL;  /* go home??? *
+    }
 
     uint16_t i;
     for (i = 0; i < table->free_count; i++) {

In directory 'Modules', file 'getpath.c', I found a call to fseek()
which is not checked for a return value < 0, indicating failure.  The
patch file below corrects this issue:

--- getpath.c.orig      2015-04-04 16:07:25.540472702 -0700
+++ getpath.c   2015-04-04 16:09:30.988416490 -0700
@@ -265,7 +265,9 @@
     int result = 0; /* meaning not found */
     char buffer[MAXPATHLEN*2+1];  /* allow extra for key, '=', etc. */
 
-    fseek(env_file, 0, SEEK_SET);
+    if (fseek(env_file, 0, SEEK_SET) < 0)
+        return result;
+   
     while (!feof(env_file)) {
         char * p = fgets(buffer, MAXPATHLEN*2, env_file);
         wchar_t tmpbuffer[MAXPATHLEN*2+1];
		 		
I am attaching the patch file(s) to this bug report...

Bill Parker (wp02855 at gmail dot com)
msg240161 - (view) Author: Bill Parker (dogbert2) * Date: 2015-04-06 17:01
Addition of file 'ffi.c.patch'...
msg240179 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2015-04-06 20:34
Thanks for the patches.  For future reference, it would be better to open separate issues for problems found in different areas of Python as there may be different reviewers and dependencies.  For example, libffi is a separate project, mirrored with some patches in the Python source repo.  You should report your proposed arm/ffi.c patch upstream: https://sourceware.org/libffi/
msg240180 - (view) Author: Bill Parker (dogbert2) * Date: 2015-04-06 20:41
Per Ned Deily, I did send 'ffi.c.patch' to the guys upstream at:

https://sourceware.org/libffi/ 

So hopefully they can review and fix it in the next release :)...

Given that Python is spread out, perhaps when a component is selected, it could display source directories and/or files (just a suggestion here)...
msg240439 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-04-10 17:51
Bill, please sign the PSF Contributor Agreement.
Info: https://www.python.org/psf/contrib/
Form: https://www.python.org/psf/contrib/contrib-form/
* appears after your name when process is completed in PSF office (a few days)
msg240440 - (view) Author: Bill Parker (dogbert2) * Date: 2015-04-10 18:01
I signed and confirmed the form returned via E-Mail...:)

On Fri, Apr 10, 2015 at 10:51 AM, Terry J. Reedy <report@bugs.python.org>
wrote:

>
> Terry J. Reedy added the comment:
>
> Bill, please sign the PSF Contributor Agreement.
> Info: https://www.python.org/psf/contrib/
> Form: https://www.python.org/psf/contrib/contrib-form/
> * appears after your name when process is completed in PSF office (a few
> days)
>
> ----------
> nosy: +terry.reedy
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue23878>
> _______________________________________
>
msg350689 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-28 21:54
New changeset e4a5e9b5bb69025eb879cb133259667241d61a1f by Victor Stinner (Zackery Spytz) in branch 'master':
bpo-23878: Remove an unneeded fseek() call in _Py_FindEnvConfigValue() (GH-15424)
https://github.com/python/cpython/commit/e4a5e9b5bb69025eb879cb133259667241d61a1f
msg350690 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-28 21:59
The fseek() call has been removed from _Py_FindEnvConfigValue (previously in getpath.c).

There is a single file called ffi.c in the master branch: ./Modules/_ctypes/libffi_osx/ffi.c And this file doesn't call calloc(). I guess that the bug was in our embedded copy of libffi (Modules/_ctypes/libffi/src/arm/ffi has been removed). Copy which has been removed in the meanwhile. If someone cares, check if all calloc() calls are checked for failure in https://sourceware.org/libffi/

From the Python point of view, all bugs described in this issue are now fixed.

Thanks for the report and thanks for the fix ;-)
History
Date User Action Args
2019-08-28 22:02:08ZackerySpytzsetpull_requests: + pull_request15241
2019-08-28 21:59:13vstinnersetstatus: open -> closed
versions: - Python 2.7, Python 3.7, Python 3.8
messages: + msg350690

resolution: fixed
stage: patch review -> resolved
2019-08-28 21:54:08vstinnersetnosy: + vstinner
messages: + msg350689
2019-08-23 15:30:08ZackerySpytzsetnosy: + ZackerySpytz

versions: + Python 2.7, Python 3.7, Python 3.8, Python 3.9, - Python 3.4
2019-08-23 15:29:03ZackerySpytzsetstage: patch review
pull_requests: + pull_request15123
2016-03-06 16:29:59berker.peksaglinkissue15948 dependencies
2015-04-10 18:01:52dogbert2setmessages: + msg240440
2015-04-10 17:51:26terry.reedysetnosy: + terry.reedy
messages: + msg240439
2015-04-06 20:41:51dogbert2setmessages: + msg240180
2015-04-06 20:34:02ned.deilysetnosy: + ned.deily
messages: + msg240179
2015-04-06 17:01:00dogbert2setfiles: + ffi.c.patch

messages: + msg240161
2015-04-06 17:00:27dogbert2create