Fix value chain use-after-free
authorTom Tromey <tom@tromey.com>
Wed, 8 Feb 2023 20:59:36 +0000 (13:59 -0700)
committerTom Tromey <tom@tromey.com>
Mon, 27 Feb 2023 22:46:31 +0000 (15:46 -0700)
commitf3d3bbbcdd8af6295458eee3b023447c13edabd3
tree7f505c42452ba1c2890b2c56e278eb53dd2a1fed
parent9b955acd7f5029345e7bd8e31a4af0e6b356f0ae
Fix value chain use-after-free

Hannes filed a bug showing a crash, where a pretty-printer written in
Python could cause a use-after-free.  He sent a patch, but I thought a
different approach was needed.

In a much earlier patch (see bug #12533), we changed the Python code
to release new values from the value chain when constructing a
gdb.Value.  The rationale for this is that if you write a command that
does a lot of computations in a loop, all the values will be kept live
by the value chain, resulting in gdb using a large amount of memory.

However, suppose a value is passed to Python from some code in gdb
that needs to use the value after the call into Python.  In this
scenario, value_to_value_object will still release the value -- and
because gdb code doesn't generally keep strong references to values (a
consequence of the ancient decision to use the value chain to avoid
memory management), this will result in a use-after-free.

This scenario can happen, as it turns out, when a value is passed to
Python for pretty-printing.  Now, normally this route boxes the value
via value_to_value_object_no_release, avoiding the problematic release
from the value chain.  However, if you then call Value.cast, the
underlying value API might return the same value, when is then
released from the chain.

This patch fixes the problem by changing how value boxing is done.
value_to_value_object no longer removes a value from the chain.
Instead, every spot in gdb that might construct new values uses a
scoped_value_mark to ensure that the requirements of bug #12533 are
met.  And, because incoming values aren't ever released from the chain
(the Value.cast one comes earlier on the chain than the
scoped_value_mark), the bug can no longer occur.  (Note that many
spots in the Python layer already take this approach, so not many
places needed to be touched.)

In the future I think we should replace the use of raw "value *" with
value_ref_ptr pretty much everywhere.  This will ensure lifetime
safety throughout gdb.

The test case in this patch comes from Hannes' original patch.  I only
made a trivial ("require") change to it.  However, while this fails
for him, I can't make it fail on this machine; nevertheless, he tried
my patch and reported the bug as being fixed.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30044
14 files changed:
gdb/python/py-finishbreakpoint.c
gdb/python/py-frame.c
gdb/python/py-lazy-string.c
gdb/python/py-prettyprint.c
gdb/python/py-symbol.c
gdb/python/py-type.c
gdb/python/py-unwind.c
gdb/python/py-value.c
gdb/python/py-xmethods.c
gdb/python/python-internal.h
gdb/python/python.c
gdb/testsuite/gdb.python/py-pp-cast.c [new file with mode: 0644]
gdb/testsuite/gdb.python/py-pp-cast.exp [new file with mode: 0644]
gdb/testsuite/gdb.python/py-pp-cast.py [new file with mode: 0644]