From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2126) id 2C5093858D32; Mon, 27 Feb 2023 22:56:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2C5093858D32 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677538584; bh=0BMEDncJG5YsqMpO/eN9Hoxj6cWlVREZPpNgLKH9/B4=; h=From:To:Subject:Date:From; b=C8gXygZvidFYvJB42SVCVa4BNrLvjZSpXi7VAL6rz/lJUXQING6qAyngWtNUgmRa2 cvPOBJmP9MJnKgWZYL0ZZVCbWPoD+f7M0p63if8ew7iy3wwW0/DLmjjXwn9zC1JKdB v46OSHw4amGX5ikXG04iAtbySYd+hRcNGNEOfviI= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Tom Tromey To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Fix value chain use-after-free X-Act-Checkin: binutils-gdb X-Git-Author: Tom Tromey X-Git-Refname: refs/heads/master X-Git-Oldrev: 9b955acd7f5029345e7bd8e31a4af0e6b356f0ae X-Git-Newrev: f3d3bbbcdd8af6295458eee3b023447c13edabd3 Message-Id: <20230227225624.2C5093858D32@sourceware.org> Date: Mon, 27 Feb 2023 22:56:24 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Df3d3bbbcdd8a= f6295458eee3b023447c13edabd3 commit f3d3bbbcdd8af6295458eee3b023447c13edabd3 Author: Tom Tromey Date: Wed Feb 8 13:59:36 2023 -0700 Fix value chain use-after-free =20 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. =20 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. =20 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. =20 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. =20 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.) =20 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. =20 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. =20 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D30044 Diff: --- gdb/python/py-finishbreakpoint.c | 4 +++ gdb/python/py-frame.c | 17 +++++++---- gdb/python/py-lazy-string.c | 9 ++++-- gdb/python/py-prettyprint.c | 2 +- gdb/python/py-symbol.c | 8 +++-- gdb/python/py-type.c | 9 ++++-- gdb/python/py-unwind.c | 11 +++++-- gdb/python/py-value.c | 54 ++++++++++++++---------------= ---- gdb/python/py-xmethods.c | 1 + gdb/python/python-internal.h | 1 - gdb/python/python.c | 8 +++-- gdb/testsuite/gdb.python/py-pp-cast.c | 35 +++++++++++++++++++++ gdb/testsuite/gdb.python/py-pp-cast.exp | 40 ++++++++++++++++++++++++ gdb/testsuite/gdb.python/py-pp-cast.py | 28 +++++++++++++++++ 14 files changed, 173 insertions(+), 54 deletions(-) diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpo= int.c index d4d129110e9..3b682f5ad36 100644 --- a/gdb/python/py-finishbreakpoint.c +++ b/gdb/python/py-finishbreakpoint.c @@ -112,6 +112,8 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_objec= t *bp_obj) =20 try { + scoped_value_mark free_values; + struct symbol *func_symbol =3D symbol_object_to_symbol (self_finishbp->func_symbol); struct value *function =3D @@ -258,6 +260,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObje= ct *kwargs) /* Remember only non-void return types. */ if (ret_type->code () !=3D TYPE_CODE_VOID) { + scoped_value_mark free_values; + /* Ignore Python errors at this stage. */ value *func_value =3D read_var_value (function, NULL, frame); self_bpfinish->function_value diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index f66d22bfa3d..ecd55d2e568 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -241,12 +241,13 @@ static PyObject * frapy_read_register (PyObject *self, PyObject *args) { PyObject *pyo_reg_id; - struct value *val =3D NULL; + PyObject *result =3D nullptr; =20 if (!PyArg_UnpackTuple (args, "read_register", 1, 1, &pyo_reg_id)) return NULL; try { + scoped_value_mark free_values; frame_info_ptr frame; int regnum; =20 @@ -257,17 +258,19 @@ frapy_read_register (PyObject *self, PyObject *args) return nullptr; =20 gdb_assert (regnum >=3D 0); - val =3D value_of_register (regnum, frame); + struct value *val =3D value_of_register (regnum, frame); =20 if (val =3D=3D NULL) PyErr_SetString (PyExc_ValueError, _("Can't read register.")); + else + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return val =3D=3D NULL ? NULL : value_to_value_object (val); + return result; } =20 /* Implementation of gdb.Frame.block (self) -> gdb.Block. @@ -482,7 +485,6 @@ frapy_read_var (PyObject *self, PyObject *args) PyObject *sym_obj, *block_obj =3D NULL; struct symbol *var =3D NULL; /* gcc-4.3.2 false warning. */ const struct block *block =3D NULL; - struct value *val =3D NULL; =20 if (!PyArg_ParseTuple (args, "O|O", &sym_obj, &block_obj)) return NULL; @@ -540,18 +542,21 @@ frapy_read_var (PyObject *self, PyObject *args) return NULL; } =20 + PyObject *result =3D nullptr; try { FRAPY_REQUIRE_VALID (self, frame); =20 - val =3D read_var_value (var, block, frame); + scoped_value_mark free_values; + struct value *val =3D read_var_value (var, block, frame); + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (val); + return result; } =20 /* Select this frame. */ diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c index 7177d74c32e..9b1205463d4 100644 --- a/gdb/python/py-lazy-string.c +++ b/gdb/python/py-lazy-string.c @@ -104,7 +104,6 @@ static PyObject * stpy_convert_to_value (PyObject *self, PyObject *args) { lazy_string_object *self_string =3D (lazy_string_object *) self; - struct value *val =3D NULL; =20 if (self_string->address =3D=3D 0) { @@ -113,10 +112,14 @@ stpy_convert_to_value (PyObject *self, PyObject *args) return NULL; } =20 + PyObject *result =3D nullptr; try { + scoped_value_mark free_values; + struct type *type =3D type_object_to_type (self_string->type); struct type *realtype; + struct value *val; =20 gdb_assert (type !=3D NULL); realtype =3D check_typedef (type); @@ -141,13 +144,15 @@ stpy_convert_to_value (PyObject *self, PyObject *args) val =3D value_at_lazy (type, self_string->address); break; } + + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (val); + return result; } =20 static void diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c index dbacb3f3fe2..29ae0205ec7 100644 --- a/gdb/python/py-prettyprint.c +++ b/gdb/python/py-prettyprint.c @@ -590,7 +590,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_= language_defn *extlang, =20 gdbpy_enter enter_py (gdbarch, language); =20 - gdbpy_ref<> val_obj (value_to_value_object_no_release (value)); + gdbpy_ref<> val_obj (value_to_value_object (value)); if (val_obj =3D=3D NULL) { print_stack_unless_memory_error (stream); diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c index b8777966c47..e6497ed473f 100644 --- a/gdb/python/py-symbol.c +++ b/gdb/python/py-symbol.c @@ -267,7 +267,6 @@ sympy_value (PyObject *self, PyObject *args) struct symbol *symbol =3D NULL; frame_info_ptr frame_info =3D NULL; PyObject *frame_obj =3D NULL; - struct value *value =3D NULL; =20 if (!PyArg_ParseTuple (args, "|O", &frame_obj)) return NULL; @@ -285,6 +284,7 @@ sympy_value (PyObject *self, PyObject *args) return NULL; } =20 + PyObject *result =3D nullptr; try { if (frame_obj !=3D NULL) @@ -301,14 +301,16 @@ sympy_value (PyObject *self, PyObject *args) was found, so we have no block to pass to read_var_value. This will yield an incorrect value when symbol is not local to FRAME_INFO (this can happen with nested functions). */ - value =3D read_var_value (symbol, NULL, frame_info); + scoped_value_mark free_values; + struct value *value =3D read_var_value (symbol, NULL, frame_info); + result =3D value_to_value_object (value); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (value); + return result; } =20 /* Given a symbol, and a symbol_object that has previously been diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c index 0a3787815ec..b68ec8d2c92 100644 --- a/gdb/python/py-type.c +++ b/gdb/python/py-type.c @@ -957,7 +957,6 @@ typy_template_argument (PyObject *self, PyObject *args) const struct block *block =3D NULL; PyObject *block_obj =3D NULL; struct symbol *sym; - struct value *val =3D NULL; =20 if (! PyArg_ParseTuple (args, "i|O", &argno, &block_obj)) return NULL; @@ -1014,16 +1013,19 @@ typy_template_argument (PyObject *self, PyObject *a= rgs) return NULL; } =20 + PyObject *result =3D nullptr; try { - val =3D value_of_variable (sym, block); + scoped_value_mark free_values; + struct value *val =3D value_of_variable (sym, block); + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (val); + return result; } =20 static PyObject * @@ -1189,6 +1191,7 @@ typy_optimized_out (PyObject *self, PyObject *args) { struct type *type =3D ((type_object *) self)->type; =20 + scoped_value_mark free_values; return value_to_value_object (value::allocate_optimized_out (type)); } =20 diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index 5b512c95dc7..ee776bf8dea 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -365,7 +365,6 @@ static PyObject * pending_framepy_read_register (PyObject *self, PyObject *args) { pending_frame_object *pending_frame =3D (pending_frame_object *) self; - struct value *val =3D NULL; int regnum; PyObject *pyo_reg_id; =20 @@ -380,25 +379,31 @@ pending_framepy_read_register (PyObject *self, PyObje= ct *args) if (!gdbpy_parse_register_id (pending_frame->gdbarch, pyo_reg_id, ®nu= m)) return nullptr; =20 + PyObject *result =3D nullptr; try { + scoped_value_mark free_values; + /* Fetch the value associated with a register, whether it's a real register or a so called "user" register, like "pc", which maps to a real register. In the past, get_frame_register_value() was used here, which did not handle the user register case. */ - val =3D value_of_register (regnum, pending_frame->frame_info); + struct value *val =3D value_of_register (regnum, + pending_frame->frame_info); if (val =3D=3D NULL) PyErr_Format (PyExc_ValueError, "Cannot read register %d from frame.", regnum); + else + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return val =3D=3D NULL ? NULL : value_to_value_object (val); + return result; } =20 /* Implementation of diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c index c61de577de1..9441a43cad8 100644 --- a/gdb/python/py-value.c +++ b/gdb/python/py-value.c @@ -1559,18 +1559,20 @@ valpy_nonzero (PyObject *self) static PyObject * valpy_invert (PyObject *self) { - struct value *val =3D NULL; + PyObject *result =3D nullptr; =20 try { - val =3D value_complement (((value_object *) self)->value); + scoped_value_mark free_values; + struct value *val =3D value_complement (((value_object *) self)->val= ue); + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (val); + return result; } =20 /* Implements left shift for value objects. */ @@ -1774,32 +1776,10 @@ valpy_float (PyObject *self) return PyFloat_FromDouble (d); } =20 -/* Returns an object for a value which is released from the all_values cha= in, - so its lifetime is not bound to the execution of a command. */ -PyObject * -value_to_value_object (struct value *val) -{ - value_object *val_obj; - - val_obj =3D PyObject_New (value_object, &value_object_type); - if (val_obj !=3D NULL) - { - val_obj->value =3D release_value (val).release (); - val_obj->next =3D nullptr; - val_obj->prev =3D nullptr; - val_obj->address =3D NULL; - val_obj->type =3D NULL; - val_obj->dynamic_type =3D NULL; - note_value (val_obj); - } - - return (PyObject *) val_obj; -} - -/* Returns an object for a value, but without releasing it from the +/* Returns an object for a value, without releasing it from the all_values chain. */ PyObject * -value_to_value_object_no_release (struct value *val) +value_to_value_object (struct value *val) { value_object *val_obj; =20 @@ -1926,21 +1906,23 @@ PyObject * gdbpy_history (PyObject *self, PyObject *args) { int i; - struct value *res_val =3D NULL; /* Initialize to appease gcc warning. = */ =20 if (!PyArg_ParseTuple (args, "i", &i)) return NULL; =20 + PyObject *result =3D nullptr; try { - res_val =3D access_value_history (i); + scoped_value_mark free_values; + struct value *res_val =3D access_value_history (i); + result =3D value_to_value_object (res_val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (res_val); + return result; } =20 /* Add a gdb.Value into GDB's history, and return (as an integer) the @@ -1988,15 +1970,23 @@ gdbpy_convenience_variable (PyObject *self, PyObjec= t *args) if (!PyArg_ParseTuple (args, "s", &varname)) return NULL; =20 + PyObject *result =3D nullptr; + bool found =3D false; try { struct internalvar *var =3D lookup_only_internalvar (varname); =20 if (var !=3D NULL) { + scoped_value_mark free_values; res_val =3D value_of_internalvar (gdbpy_enter::get_gdbarch (), var); if (res_val->type ()->code () =3D=3D TYPE_CODE_VOID) res_val =3D NULL; + else + { + found =3D true; + result =3D value_to_value_object (res_val); + } } } catch (const gdb_exception &except) @@ -2004,10 +1994,10 @@ gdbpy_convenience_variable (PyObject *self, PyObjec= t *args) GDB_PY_HANDLE_EXCEPTION (except); } =20 - if (res_val =3D=3D NULL) + if (result =3D=3D nullptr && !found) Py_RETURN_NONE; =20 - return value_to_value_object (res_val); + return result; } =20 /* Set the value of a convenience variable. */ diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c index 2db6793b507..d9fa355e4cb 100644 --- a/gdb/python/py-xmethods.c +++ b/gdb/python/py-xmethods.c @@ -423,6 +423,7 @@ python_xmethod_worker::do_get_result_type (value *obj, return EXT_LANG_RC_OK; } =20 + scoped_value_mark free_values; obj_type =3D check_typedef (obj->type ()); this_type =3D check_typedef (type_object_to_type (m_this_type)); if (obj_type->code () =3D=3D TYPE_CODE_PTR) diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 8fb09796b15..55bbe78a7a5 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -437,7 +437,6 @@ PyObject *symbol_to_symbol_object (struct symbol *sym); PyObject *block_to_block_object (const struct block *block, struct objfile *objfile); PyObject *value_to_value_object (struct value *v); -PyObject *value_to_value_object_no_release (struct value *v); PyObject *type_to_type_object (struct type *); PyObject *frame_info_to_frame_object (frame_info_ptr frame); PyObject *symtab_to_linetable_object (PyObject *symtab); diff --git a/gdb/python/python.c b/gdb/python/python.c index 1ed13f2789b..fb3d975c6d2 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -970,22 +970,24 @@ static PyObject * gdbpy_parse_and_eval (PyObject *self, PyObject *args) { const char *expr_str; - struct value *result =3D NULL; =20 if (!PyArg_ParseTuple (args, "s", &expr_str)) return NULL; =20 + PyObject *result =3D nullptr; try { gdbpy_allow_threads allow_threads; - result =3D parse_and_eval (expr_str); + scoped_value_mark free_values; + struct value *val =3D parse_and_eval (expr_str); + result =3D value_to_value_object (val); } catch (const gdb_exception &except) { GDB_PY_HANDLE_EXCEPTION (except); } =20 - return value_to_value_object (result); + return result; } =20 /* Implementation of gdb.invalidate_cached_frames. */ diff --git a/gdb/testsuite/gdb.python/py-pp-cast.c b/gdb/testsuite/gdb.pyth= on/py-pp-cast.c new file mode 100644 index 00000000000..72716a79dd4 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-cast.c @@ -0,0 +1,35 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +typedef int pp_int; + +int break_function() +{ + return 0; +} + +struct container +{ + pp_int p_i; + int i; +}; + +int main() +{ + struct container c =3D { 10, 5 }; + return break_function(); +} diff --git a/gdb/testsuite/gdb.python/py-pp-cast.exp b/gdb/testsuite/gdb.py= thon/py-pp-cast.exp new file mode 100644 index 00000000000..8a6f2976046 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-cast.exp @@ -0,0 +1,40 @@ +# Copyright (C) 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test casting of a gdb.Value inside a pretty printer. + +load_lib gdb-python.exp + +require allow_python_tests + +standard_testfile + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { + return -1 +} + +if ![runto break_function] { + return -1 +} + +set remote_python_file [gdb_remote_download host \ + ${srcdir}/${subdir}/${testfile}.py] + +gdb_test_no_output "source ${remote_python_file}" \ + "source ${testfile}.py" + +gdb_test "up" "#1.*main.*" + +gdb_test "info locals" "c =3D {p_i =3D 10p, i =3D 5}" diff --git a/gdb/testsuite/gdb.python/py-pp-cast.py b/gdb/testsuite/gdb.pyt= hon/py-pp-cast.py new file mode 100644 index 00000000000..b171a919c70 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-cast.py @@ -0,0 +1,28 @@ +# Copyright (C) 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + + +class PpIntPrinter(object): + def __init__(self, val): + self.val =3D val + + def to_string(self): + val =3D self.val.cast(self.val.type) + return "%dp" % int(val) + + +pp =3D gdb.printing.RegexpCollectionPrettyPrinter("pp-cast") +pp.add_printer("pp_int", "^pp_int$", PpIntPrinter) +gdb.printing.register_pretty_printer(gdb.current_objfile(), pp)