Missing sanity checks for various C library function calls...

[dogbert2]

BPO	23878
Nosy	@terryjreedy, @vstinner, @ned-deily, @ZackerySpytz
PRs	bpo-23878: Remove an unneeded fseek() call in _Py_FindEnvConfigValue() #15424 bpo-23878: Remove the key parameter of _Py_FindEnvConfigValue() #15555
Files	getpath.c.patch ffi.c.patch: patch file (diff -u) for this bug

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-08-28.21:59:13.862>
created_at = <Date 2015-04-06.17:00:27.258>
labels = ['interpreter-core', 'type-bug', '3.9']
title = 'Missing sanity checks for various C library function calls...'
updated_at = <Date 2019-08-28.22:02:08.513>
user = 'https://bugs.python.org/dogbert2'

bugs.python.org fields:

activity = <Date 2019-08-28.22:02:08.513>
actor = 'ZackerySpytz'
assignee = 'none'
closed = True
closed_date = <Date 2019-08-28.21:59:13.862>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2015-04-06.17:00:27.258>
creator = 'dogbert2'
dependencies = []
files = ['38848', '38849']
hgrepos = []
issue_num = 23878
keywords = ['patch']
message_count = 8.0
messages = ['240160', '240161', '240179', '240180', '240439', '240440', '350689', '350690']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'vstinner', 'ned.deily', 'dogbert2', 'ZackerySpytz']
pr_nums = ['15424', '15555']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23878'
versions = ['Python 3.9']

[dogbert2]

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)

[dogbert2]

Addition of file 'ffi.c.patch'...

[ned-deily]

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/

[dogbert2]

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)...

[terryjreedy]

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)

[dogbert2]

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\>

---

[vstinner]

New changeset e4a5e9b by Victor Stinner (Zackery Spytz) in branch 'master':
bpo-23878: Remove an unneeded fseek() call in _Py_FindEnvConfigValue() (GH-15424)
e4a5e9b

[vstinner]

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 ;-)