Incomplete tuple created by PyTuple_New() and accessed via the GC can trigged a crash

[pxd]

BPO	15108
Nosy	@arigo, @rhettinger, @jcea, @amauryfa, @pitrou, @vstinner, @corona10, @pablogsal, @erlend-aasland, @iritkatriel
PRs	bpo-15108: Prevent accessing the result tuple from Python in PySequence_Tuple #24510

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-02-22.09:15:10.010>
created_at = <Date 2012-06-19.21:52:13.870>
labels = ['type-bug', '3.8', '3.9', '3.10', 'extension-modules', 'invalid']
title = 'Incomplete tuple created by PyTuple_New() and accessed via the GC can trigged a crash'
updated_at = <Date 2021-02-22.09:15:10.009>
user = 'https://bugs.python.org/pxd'

bugs.python.org fields:

activity = <Date 2021-02-22.09:15:10.009>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2021-02-22.09:15:10.010>
closer = 'pablogsal'
components = ['Extension Modules']
creation = <Date 2012-06-19.21:52:13.870>
creator = 'pxd'
dependencies = []
files = []
hgrepos = []
issue_num = 15108
keywords = ['patch']
message_count = 22.0
messages = ['163224', '163225', '163226', '164257', '164263', '164273', '164282', '164285', '228344', '228357', '386841', '387029', '387033', '387034', '387036', '387068', '387070', '387079', '387204', '387224', '387492', '387506']
nosy_count = 13.0
nosy_names = ['arigo', 'rhettinger', 'jcea', 'ghaering', 'amaury.forgeotdarc', 'pitrou', 'vstinner', 'lkraav', 'pxd', 'corona10', 'pablogsal', 'erlendaasland', 'iritkatriel']
pr_nums = ['24510']
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15108'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

Linked PRs

• GH-59313: Do not allow incomplete tuples, or tuples with NULLs. #117747

[pxd]

Hi,

Sporadically, while running an sqlite3 query, the above error is seen.
In order to debug, I modified Objects/tupleobject.c, PyTuple_SetItem() as follows:

	if (!PyTuple_Check(op) || op->ob_refcnt != 1) {
		Py_XDECREF(newitem);
		/*
		 * Temp: Bug XYZ Generate core so that we can debug
		 * 
		 * PyErr_BadInternalCall();
		 * return -1;
		*/
		char errmsg[200];
		sprintf(errmsg, "Bug XYZ: PyTuple_Check(op) = %d "
			"op->ob_refcnt = %d; see core\n", PyTuple_Check(op),
			op->ob_refcnt);
		Py_FatalError(errmsg); 
	}

This generates a core with the following bt. Showing the top few frames only:
(gdb) bt
#0 0x0000000800acd3fc in thr_kill () at thr_kill.S:3
#1 0x0000000800b5e283 in abort () at /build/mnt/src/lib/libc/stdlib/abort.c:65
#2 0x0000000000494acf in Py_FatalError (msg=Variable "msg" is not available.
) at ./../Python/pythonrun.c:1646
#3 0x000000000044e740 in PyTuple_SetItem (op=0x80c6e6308, i=16, newitem=0x80a80d780) at ./../Objects/tupleobject.c:128
#4 0x0000000807298866 in _pysqlite_fetch_one_row (self=0x80b846e48) at _sqlite/cursor.c:402
#5 0x0000000807298bf5 in pysqlite_cursor_iternext (self=0x80b846e48) at _sqlite/cursor.c:898
#6 0x0000000000476943 in PyEval_EvalFrameEx (f=0x80a94d420, throwflag=Variable "throwflag" is not available.
) at ./../Python/ceval.c:2237
#7 0x000000000047acbf in PyEval_EvalFrameEx (f=0x80a94b820, throwflag=Variable "throwflag" is not available.
) at ./../Python/ceval.c:3765
#8 0x000000000047bf09 in PyEval_EvalCodeEx (co=0x808d2ac60, globals=Variable "globals" is not available.
) at ./../Python/ceval.c:2942
...
(gdb) fr 4
#4 0x0000000807298866 in _pysqlite_fetch_one_row (self=0x80b846e48) at _sqlite/cursor.c:402
402 PyTuple_SetItem(row, i, converted);
Current language: auto; currently c
(gdb) l
397 converted = buffer;
398 }

(gdb) p *(PyTupleObject *)row
$11 = {ob_refcnt = 2, ob_type = 0x60fee0, ob_size = 22, ob_item = {0x80a534030}}

'row' was allocated via PyTuple_New()
but, somehow its refcount has become 2 while setting the 16th item!!!

Is this a known issue? If not, what can I do to debug.

Thanks,
Pankaj

[jcea]

Could you possibly reproduce this in 2.7, 3.2 and/or "default" (future 3.3)?.

Python 2.6 support is over.

[pxd]

sorry, 2.7, 3.2 is not an option currently but I am hoping someone can
provide enough info to help probe this more efficiently. There seem to be references to this issue on the web but no root-cause.

[pxd]

I believe I have found the root-cause for this issue.

It is occurring due to the use of the garbage collector in another “memMonitor” thread (we run it periodically to get stats on objects, track mem leaks, etc). Since _pysqlite_fetch_one_row() releases the GIL before calling PyTuple_SetItem(), if the memMonitor is scheduled to run and, say, calls gc.get_objects(), it increments the refcount on all tracked objects (via append_objects()->PyList_Append()->app1()->PY_INCREF()). I have stack traces to confirm. This seems to rule out the use of gc methods (such as get_objects(), get_referrers/referents()) in multi-threaded programs or have them handle SystemError arising from such usage. Agree?

[amauryfa]

Thanks for the analysis!
This is quite similar to bpo-793822: gc.get_referrers() can access unfinished tuples. The difference here is that what breaks is not the monitoring tool, but the "main" program!

Here is a simple script inspired from the original bug; PySequence_Tuple() uses PyTuple_SET_ITEM() which is a macro without the ob_refcnt check, but we can make it call _PyTuple_Resize() and fail there. All versions of CPython are affected:

import gc
TAG = object()

def monitor():
 lst = [x for x in gc.get_referrers(TAG)
 if isinstance(x, tuple)]
 t = lst[0] # this *is* the result tuple
 print(t) # full of nulls !
 return t # Keep it alive for some time

def my_iter():
 yield TAG # 'tag' gets stored in the result tuple
 t = monitor()
 for x in range(10):
 yield x # SystemError when the tuple needs to be resized

tuple(my_iter())

[pitrou]

I wonder why _pysqlite_fetch_one_row() releases the GIL around sqlite3_column_type(). By its name, it doesn't sound like an expensive function.

Another workaround would be to call PyTuple_SET_ITEM instead of PyTuple_SetItem.

[pxd]

Wondering the same thing myself, and yes sqlite3_column_type() by itself doesn't seem expensive. I assumed in general it was to allow more responsiveness for apps with huge number of columns (i.e. large tuple size). But we have about 20-25 columns and so I was going to try removing it and seeing the
results. In any case, it seems, fewer GIL acquire/releases will help with throughput.
Are there any guidelines on when GIL should be released?

Re PyTuple_SET_ITEM...yes that's also a possibility but it would then hide genuine bugs.

[pitrou]

Are there any guidelines on when GIL should be released?

The GIL should be released:

• for CPU-heavy external functions (e.g. compression, cryptography)
• for external functions which wait for I/O

Re PyTuple_SET_ITEM...yes that's also a possibility but it would then
hide genuine bugs.

Well, as long as your monitor only increfs the tuple and doesn't mutate
it, there shouldn't be any problem. We use PyTuple_SET_ITEM in many
other places.

[lkraav]

I may be seeing this running tracd-1.1.2b1 on python 2.7.7

http://trac.edgewall.org/ticket/11772

[pitrou]

What is tracd doing? This issue should only appear when calling one of the debugging functions in the gc module, AFAICT.

[iritkatriel]

Still happening in 3.10:

Python 3.10.0a5+ (heads/master:bf2e7e55d7, Feb 11 2021, 23:09:25) [MSC v.1928 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import gc
>>> TAG = object()
>>>
>>> def monitor():
... lst = [x for x in gc.get_referrers(TAG)
... if isinstance(x, tuple)]
... t = lst[0] # this *is* the result tuple
... print(t) # full of nulls !
... return t # Keep it alive for some time
...
>>> def my_iter():
... yield TAG # 'tag' gets stored in the result tuple
... t = monitor()
... for x in range(10):
... yield x # SystemError when the tuple needs to be resized
...
>>> tuple(my_iter())
(<object object at 0x00000217225091B0>, <NULL>, <NULL>, <NULL>, <NULL>, <NULL>, <NULL>, <NULL>, <NULL>, <NULL>)
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
SystemError: C:\Users\User\src\cpython-dev\Objects\tupleobject.c:963: bad argument to internal function
>>>