Skip to content

Commit

Permalink
libdrgn/python: fix segfault on del prog.language and more
Browse files Browse the repository at this point in the history
None of our setter functions handle deletions, which pass the value as
NULL. This results in a segfault when attempting to access the value.
Fix them all with a new convenience macro.

Fixes: 50e4ac8 ("libdrgn: allow overriding program default language")
Fixes: 4e83130 ("Introduce module and debug info finder APIs")
Signed-off-by: Omar Sandoval <[email protected]>
  • Loading branch information
osandov committed Jan 23, 2025
1 parent 64d82dd commit 4738ddf
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 0 deletions.
12 changes: 12 additions & 0 deletions libdrgn/python/drgnpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ static inline PyObject *PyObject_CallOneArg(PyObject *callable, PyObject *arg)
Py_RETURN_FALSE; \
} while (0)

/**
* Return from a PyGetSetDef setter with an error if attempting to delete the
* attribute.
*/
#define SETTER_NO_DELETE(name, value) do { \
if (!(value)) { \
PyErr_Format(PyExc_AttributeError, \
"can't delete '%s' attribute", (name)); \
return -1; \
} \
} while (0)

static inline void pydecrefp(void *p)
{
Py_XDECREF(*(PyObject **)p);
Expand Down
3 changes: 3 additions & 0 deletions libdrgn/python/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ static PyObject *Module_get_address_range(Module *self, void *arg)

static int Module_set_address_range(Module *self, PyObject *value, void *arg)
{
SETTER_NO_DELETE("address_range", value);
struct drgn_error *err;
if (value == Py_None) {
err = drgn_module_set_address_range(self->module, -1, -1);
Expand Down Expand Up @@ -258,6 +259,7 @@ static PyObject *Module_get_build_id(Module *self, void *arg)

static int Module_set_build_id(Module *self, PyObject *value, void *arg)
{
SETTER_NO_DELETE("build_id", value);
struct drgn_error *err;
if (value == Py_None) {
err = drgn_module_set_build_id(self->module, NULL, 0);
Expand Down Expand Up @@ -300,6 +302,7 @@ static PyObject *Module_get_##which##_file_status(Module *self, void *arg) \
static int Module_set_##which##_file_status(Module *self, PyObject *value, \
void *arg) \
{ \
SETTER_NO_DELETE(#which, value); \
if (!PyObject_TypeCheck(value, \
(PyTypeObject *)ModuleFileStatus_class)) { \
PyErr_SetString(PyExc_TypeError, \
Expand Down
2 changes: 2 additions & 0 deletions libdrgn/python/program.c
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,7 @@ static PyObject *Program_get_debug_info_path(Program *self, void *arg)

static int Program_set_debug_info_path(Program *self, PyObject *value, void *arg)
{
SETTER_NO_DELETE("debug_info_path", value);
const char *path;
if (value == Py_None) {
path = NULL;
Expand Down Expand Up @@ -1909,6 +1910,7 @@ static PyObject *Program_get_language(Program *self, void *arg)

static int Program_set_language(Program *self, PyObject *value, void *arg)
{
SETTER_NO_DELETE("language", value);
if (!PyObject_TypeCheck(value, &Language_type)) {
PyErr_SetString(PyExc_TypeError, "language must be Language");
return -1;
Expand Down
12 changes: 12 additions & 0 deletions tests/test_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@ def test_address_range_invalid(self):
with self.assertRaisesRegex(ValueError, "invalid module address range"):
module.address_range = (2**64 - 1, 2**64 - 1)

def test_address_range_del(self):
module = Program().extra_module("/foo/bar", create=True)[0]
with self.assertRaises(AttributeError):
del module.address_range

def test_build_id(self):
module = Program().extra_module("/foo/bar", create=True)[0]

Expand All @@ -376,6 +381,11 @@ def test_build_id_invalid_empty(self):
with self.assertRaisesRegex(ValueError, "build ID cannot be empty"):
module.build_id = b""

def test_build_id_del(self):
module = Program().extra_module("/foo/bar", create=True)[0]
with self.assertRaises(AttributeError):
del module.build_id

def test_find_by_address(self):
prog = Program()
module1 = prog.extra_module("/foo/bar", create=True)[0]
Expand Down Expand Up @@ -456,6 +466,8 @@ def _test_file_status(self, which):
setattr(module, status_attr, ModuleFileStatus.WANT)
self.assertEqual(getattr(module, status_attr), ModuleFileStatus.WANT)

self.assertRaises(AttributeError, delattr, module, status_attr)

def test_loaded_file_status(self):
self._test_file_status("loaded")

Expand Down
4 changes: 4 additions & 0 deletions tests/test_program.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ def test_language(self):
TypeError, "language must be Language", setattr, prog, "language", "CPP"
)

def test_language_del(self):
with self.assertRaises(AttributeError):
del Program().language


class TestMemory(TestCase):
def test_simple_read(self):
Expand Down

0 comments on commit 4738ddf

Please sign in to comment.