Skip to content

Commit

Permalink
Close #16742: Fix misuse of memory allocations in PyOS_Readline()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vstinner committed Oct 10, 2013
1 parent 32c0229 commit 6a84e08
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 8 deletions.
8 changes: 8 additions & 0 deletions Doc/c-api/veryhigh.rst
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ the same library that the Python runtime is using.
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)
Expand Down
6 changes: 6 additions & 0 deletions Doc/whatsnew/3.4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -587,3 +587,9 @@ that may require changes to your code.
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`.

5 changes: 5 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ Projected release date: 2013-10-20
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
Expand Down
4 changes: 2 additions & 2 deletions Modules/readline.c
Original file line number Diff line number Diff line change
Expand Up @@ -1176,7 +1176,7 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)

/* 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)
Expand Down Expand Up @@ -1204,7 +1204,7 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
/* 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';
Expand Down
26 changes: 20 additions & 6 deletions Parser/myreadline.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,22 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
{
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 */
Expand All @@ -140,7 +144,7 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
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();
Expand All @@ -151,7 +155,7 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
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();
Expand All @@ -174,7 +178,8 @@ char *(*PyOS_ReadlineFunctionPointer)(FILE *, FILE *, char *);
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,
Expand Down Expand Up @@ -221,5 +226,14 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)

_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;
}

0 comments on commit 6a84e08

Please sign in to comment.