[Python-checkins] cpython: Close #16742: Fix misuse of memory allocations in PyOS_Readline()

[victor.stinner]

http://hg.python.org/cpython/rev/98dbe677dfe7
changeset:   86198:98dbe677dfe7
user:        Victor Stinner <victor.stinner at gmail.com>
date:        Thu Oct 10 16:18:20 2013 +0200
summary:
  Close #16742: Fix misuse of memory allocations in PyOS_Readline()

The GIL must be held to call PyMem_Malloc(), whereas PyOS_Readline() releases
the GIL to read input.

The result of the C callback PyOS_ReadlineFunctionPointer must now be a string
allocated by PyMem_RawMalloc() or PyMem_RawRealloc() (or NULL if an error
occurred), instead of a string allocated by PyMem_Malloc() or PyMem_Realloc().

Fixing this issue was required to setup a hook on PyMem_Malloc(), for example
using the tracemalloc module.

PyOS_Readline() copies the result of PyOS_ReadlineFunctionPointer() into a new
buffer allocated by PyMem_Malloc(). So the public API of PyOS_Readline() does
not change.

files:
  Doc/c-api/veryhigh.rst |   8 ++++++++
  Doc/whatsnew/3.4.rst   |   6 ++++++
  Misc/NEWS              |   5 +++++
  Modules/readline.c     |   4 ++--
  Parser/myreadline.c    |  26 ++++++++++++++++++++------
  5 files changed, 41 insertions(+), 8 deletions(-)


diff --git a/Doc/c-api/veryhigh.rst b/Doc/c-api/veryhigh.rst
--- a/Doc/c-api/veryhigh.rst
+++ b/Doc/c-api/veryhigh.rst
@@ -166,6 +166,14 @@
    resulting string.  For example, The :mod:`readline` module sets
    this hook to provide line-editing and tab-completion features.
 
+   The result must be a string allocated by :c:func:`PyMem_RawMalloc` or
+   :c:func:`PyMem_RawRealloc`, or *NULL* if an error occurred.
+
+   .. versionchanged:: 3.4
+      The result must be allocated by :c:func:`PyMem_RawMalloc` or
+      :c:func:`PyMem_RawRealloc`, instead of being allocated by
+      :c:func:`PyMem_Malloc` or :c:func:`PyMem_Realloc`.
+
 
 .. c:function:: struct _node* PyParser_SimpleParseString(const char *str, int start)
 
diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst
--- a/Doc/whatsnew/3.4.rst
+++ b/Doc/whatsnew/3.4.rst
@@ -587,3 +587,9 @@
   attribute in the chain referring to the innermost function. Introspection
   libraries that assumed the previous behaviour was intentional can use
   :func:`inspect.unwrap` to gain equivalent behaviour.
+
+* (C API) The result of the :c:var:`PyOS_ReadlineFunctionPointer` callback must
+  now be a string allocated by :c:func:`PyMem_RawMalloc` or
+  :c:func:`PyMem_RawRealloc`, or *NULL* if an error occurred, instead of a
+  string allocated by :c:func:`PyMem_Malloc` or :c:func:`PyMem_Realloc`.
+
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@
 Core and Builtins
 -----------------
 
+- Issue #16742: The result of the C callback PyOS_ReadlineFunctionPointer must
+  now be a string allocated by PyMem_RawMalloc() or PyMem_RawRealloc() (or NULL
+  if an error occurred), instead of a string allocated by PyMem_Malloc() or
+  PyMem_Realloc().
+
 - Issue #19199: Remove ``PyThreadState.tick_counter`` field
 
 - Fix macro expansion of _PyErr_OCCURRED(), and make sure to use it in at
diff --git a/Modules/readline.c b/Modules/readline.c
--- a/Modules/readline.c
+++ b/Modules/readline.c
@@ -1176,7 +1176,7 @@
 
     /* We got an EOF, return a empty string. */
     if (p == NULL) {
-        p = PyMem_Malloc(1);
+        p = PyMem_RawMalloc(1);
         if (p != NULL)
             *p = '\0';
         RESTORE_LOCALE(saved_locale)
@@ -1204,7 +1204,7 @@
     /* Copy the malloc'ed buffer into a PyMem_Malloc'ed one and
        release the original. */
     q = p;
-    p = PyMem_Malloc(n+2);
+    p = PyMem_RawMalloc(n+2);
     if (p != NULL) {
         strncpy(p, q, n);
         p[n] = '\n';
diff --git a/Parser/myreadline.c b/Parser/myreadline.c
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -113,18 +113,22 @@
 {
     size_t n;
     char *p, *pr;
+
     n = 100;
-    if ((p = (char *)PyMem_MALLOC(n)) == NULL)
+    p = (char *)PyMem_RawMalloc(n);
+    if (p == NULL)
         return NULL;
+
     fflush(sys_stdout);
     if (prompt)
         fprintf(stderr, "%s", prompt);
     fflush(stderr);
+
     switch (my_fgets(p, (int)n, sys_stdin)) {
     case 0: /* Normal case */
         break;
     case 1: /* Interrupt */
-        PyMem_FREE(p);
+        PyMem_RawFree(p);
         return NULL;
     case -1: /* EOF */
     case -2: /* Error */
@@ -140,7 +144,7 @@
             PyErr_SetString(PyExc_OverflowError, "input line too long");
             return NULL;
         }
-        pr = (char *)PyMem_REALLOC(p, n + incr);
+        pr = (char *)PyMem_RawRealloc(p, n + incr);
         if (pr == NULL) {
             PyMem_FREE(p);
             PyErr_NoMemory();
@@ -151,7 +155,7 @@
             break;
         n += strlen(p+n);
     }
-    pr = (char *)PyMem_REALLOC(p, n+1);
+    pr = (char *)PyMem_RawRealloc(p, n+1);
     if (pr == NULL) {
         PyMem_FREE(p);
         PyErr_NoMemory();
@@ -174,7 +178,8 @@
 char *
 PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 {
-    char *rv;
+    char *rv, *res;
+    size_t len;
 
     if (_PyOS_ReadlineTState == PyThreadState_GET()) {
         PyErr_SetString(PyExc_RuntimeError,
@@ -221,5 +226,14 @@
 
     _PyOS_ReadlineTState = NULL;
 
-    return rv;
+    if (rv == NULL)
+        return NULL;
+
+    len = strlen(rv) + 1;
+    res = PyMem_Malloc(len);
+    if (res != NULL)
+        memcpy(res, rv, len);
+    PyMem_RawFree(rv);
+
+    return res;
 }

-- 
Repository URL: http://hg.python.org/cpython