Description

Eclips4

opened on Feb 16, 2023

Issue body actions

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

• gh-101952: Fix possible segfault in BUILD_SET opcode #101958

Activity

1. Eclips4

   added

   type-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

   on Feb 16, 2023
2. arhadthedev

   added

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   on Feb 16, 2023
3. kumaraditya303 commented on Feb 16, 2023

   

   kumaraditya303

   on Feb 16, 2023

   Contributor

   More actions

   cc @gvanrossum Appears to be introduced in #100912
4. gvanrossum commented on Feb 16, 2023

   

   gvanrossum

   on Feb 16, 2023

   Member

   More actions

   Yeah, the original code had

   if (set == NULL)
                   goto error;
   

   which somehow got lost.

   How can I repro this?
5. gvanrossum commented on Feb 16, 2023

   

   gvanrossum

   on Feb 16, 2023

   Member

   More actions

   If someone could come up with a fix I'll review it, that's the quickest way.
6. Eclips4 commented on Feb 16, 2023

   

   Eclips4

   on Feb 16, 2023

   MemberAuthor

   More actions

   I can fix it, but i don't know yet how to reproduce it. I was just looking through the bytecodes.c file =)
7. gvanrossum commented on Feb 16, 2023

   

   gvanrossum

   on Feb 16, 2023

   Member

   More actions

   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).

   Reacted by Kirill Podoprigora
8. Eclips4 commented on Feb 16, 2023

   

   Eclips4

   on Feb 16, 2023

   MemberAuthor

   More actions

   Okay, i'll soon send a PR
9. Eclips4

   added a commit that references this issue on Feb 16, 2023

   pythongh-101952: Fix possible segfault

   8a97285
10. gvanrossum

    closed this as completedin #101958on Feb 16, 2023
11. gvanrossum

    added a commit that references this issue on Feb 16, 2023

    gh-101952: Fix possible segfault in `BUILD_SET` opcode (#101958)

    68bd8c5

Issue actions