• Tom Lane's avatar
    Fix access-to-already-freed-memory issue in plpython's error handling. · 7e3bb080
    Tom Lane authored
    PLy_elog() could attempt to access strings that Python had already freed,
    because the strings that PLy_get_spi_error_data() returns are simply
    pointers into storage associated with the error "val" PyObject.  That's
    fine at the instant PLy_get_spi_error_data() returns them, but just after
    that PLy_traceback() intentionally releases the only refcount on that
    object, allowing it to be freed --- so that the strings we pass to
    ereport() are dangling pointers.
    
    In principle this could result in garbage output or a coredump.  In
    practice, I think the risk is pretty low, because there are no Python
    operations between where we decrement that refcount and where we use the
    strings (and copy them into PG storage), and thus no reason for Python
    to recycle the storage.  Still, it's clearly hazardous, and it leads to
    Valgrind complaints when running under a Valgrind that hasn't been
    lobotomized to ignore Python memory allocations.
    
    The code was a mess anyway: we fetched the error data out of Python
    (clearing Python's error indicator) with PyErr_Fetch, examined it, pushed
    it back into Python with PyErr_Restore (re-setting the error indicator),
    then immediately pulled it back out with another PyErr_Fetch.  Just to
    confuse matters even more, there were some gratuitous-and-yet-hazardous
    PyErr_Clear calls in the "examine" step, and we didn't get around to doing
    PyErr_NormalizeException until after the second PyErr_Fetch, making it even
    less clear which object was being manipulated where and whether we still
    had a refcount on it.  (If PyErr_NormalizeException did substitute a
    different "val" object, it's possible that the problem could manifest for
    real, because then we'd be doing assorted Python stuff with no refcount
    on the object we have string pointers into.)
    
    So, rearrange all that into some semblance of sanity, and don't decrement
    the refcount on the Python error objects until the end of PLy_elog().
    In HEAD, I failed to resist the temptation to reformat some messy bits
    from 5c3c3cd0 along the way.
    
    Back-patch as far as 9.2, because the code is substantially the same
    that far back.  I believe that 9.1 has the bug as well; but the code
    around it is rather different and I don't want to take a chance on
    breaking something for what seems a low-probability problem.
    7e3bb080
plpy_elog.c 15.1 KB