Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set: BUILD_SET opcode can be failed with segfault #101952

Closed
Eclips4 opened this issue Feb 16, 2023 · 6 comments · Fixed by #101958
Closed

Set: BUILD_SET opcode can be failed with segfault #101952

Eclips4 opened this issue Feb 16, 2023 · 6 comments · Fixed by #101958
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@Eclips4
Copy link
Member

Eclips4 commented Feb 16, 2023 •
edited by bedevere-bot
Loading

cpython/Python/generated_cases.c.h

Lines 1648 to 1667 in 36b139a

TARGET(BUILD_SET) {
PyObject **values = &PEEK(oparg);
PyObject *set;
set = PySet_New(NULL);
int err = 0;
for (int i = 0; i < oparg; i++) {
PyObject *item = values[i];
if (err == 0)
err = PySet_Add(set, item);
Py_DECREF(item);
}
if (err != 0) {
Py_DECREF(set);
if (true) { STACK_SHRINK(oparg); goto error; }
}
STACK_SHRINK(oparg);
STACK_GROW(1);
POKE(1, set);
DISPATCH();
}

&

cpython/Python/bytecodes.c

Lines 1303 to 1316 in 36b139a

inst(BUILD_SET, (values[oparg] -- set)) {
set = PySet_New(NULL);
int err = 0;
for (int i = 0; i < oparg; i++) {
PyObject *item = values[i];
if (err == 0)
err = PySet_Add(set, item);
Py_DECREF(item);
}
if (err != 0) {
Py_DECREF(set);
ERROR_IF(true, error);
}
}

Doesn't take in account case, when PySet_New(NULL) returns NULL.
We are checking that PySet_Add doesn't return a non-zero(-1) value.
But, PySet_Add has a check, that first argument is a subclass of set. Which fails, if we will pass (PyObject *) NULL as first argument. Why?

#define PySet_Check(ob) \
    (Py_IS_TYPE((ob), &PySet_Type) || \
    PyType_IsSubtype(Py_TYPE(ob), &PySet_Type))

PySet_Add uses this macross. But, Py_TYPE will be failed with segfault when try to access ob_type of (PyObject *) NULL.

Implementation of Py_TYPE:

static inline PyTypeObject* Py_TYPE(PyObject *ob) {
    return ob->ob_type;
}
(gdb) call (PyObject *) NULL
$1 = (PyObject *) 0x0
(gdb) call $1->ob_type
Cannot access memory at address 0x8

So, we should add check, that value of PySet_New is not-null.

Linked PRs
@Eclips4 Eclips4 added the type-crash A hard crash of the interpreter, possibly with a core dump label Feb 16, 2023
@arhadthedev arhadthedev added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 16, 2023
@kumaraditya303
Copy link
Contributor

cc @gvanrossum Appears to be introduced in #100912

@gvanrossum
Copy link
Member

Yeah, the original code had

if (set == NULL)
                goto error;

which somehow got lost.

How can I repro this?

@gvanrossum
Copy link
Member

If someone could come up with a fix I'll review it, that's the quickest way.

@Eclips4
Copy link
Member Author

Eclips4 commented Feb 16, 2023

I can fix it, but i don't know yet how to reproduce it. I was just looking through the bytecodes.c file =)

@gvanrossum
Copy link
Member

It's probably hard to repro, because the only way PySet_New() can error is if it runs out of memory. I think restoring those two lines will do it (nothing has been popped off the stack at that point yet).

@Eclips4
Copy link
Member Author

Eclips4 commented Feb 16, 2023

Okay, i'll soon send a PR

Eclips4 added a commit to Eclips4/cpython that referenced this issue Feb 16, 2023
@Eclips4
pythongh-101952: Fix possible segfault
8a97285
@Eclips4 Eclips4 mentioned this issue Feb 16, 2023
Merged
gvanrossum pushed a commit that referenced this issue Feb 16, 2023
@Eclips4
gh-101952: Fix possible segfault in BUILD_SET opcode (#101958)
68bd8c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-101952: Fix possible segfault in BUILD_SET opcode Eclips4/cpython
4 participants
@gvanrossum @arhadthedev @kumaraditya303 @Eclips4