New submission from Yonatan Goldschmidt <yon.goldschm...@gmail.com>:
I encountered a crash using rpyc. Since rpyc is pure-Python, I guessed the
problem is with Python itself.
Originally tested on v3.7, the rest of this issue is based on v3.9.0a2
(compiling on an Ubuntu 18.04 docker).
I narrowed down the rpyc-based snippet to this:
# server side
class X: pass
x_instance = X()
from rpyc.core import SlaveService
from rpyc.utils.classic import DEFAULT_SERVER_PORT
from rpyc.utils.server import ThreadedServer
t = ThreadedServer(SlaveService, port=DEFAULT_SERVER_PORT, reuse_addr=True)
t.start()
# client side
import rpyc
conn = rpyc.classic.connect("localhost")
x = conn.modules.__main__.x_instance
y = x.__class__
issubclass(y, int)
Client side gets a SIGSEGV in `_PyObject_LookupAttr`, dereferencing an invalid
`tp` pointer read from a posioned `v` object.
After some reference count debugging, I found that for the rpyc `y` object (in
the client code), accessing `__bases__` returns a tuple with refcount=1, and it
has a single item whose refcount is 1 as well.
abstract_issubclass() calls abstract_get_bases() to get this refcount=1 tuple,
and in the fastpath for single inheritance (tuple size = 1) it loads the single
item from the tuple (`derived = PyTuple_GET_ITEM(bases, 0)`) and then
decrements the refcount on the tuple, effectively deallocating the tuple and
the `derived` object (whose only reference was from the tuple).
I tried to mimic the Python magic rpyc does to get the same crash without rpyc,
and reached the following snippet (which doesn't exhibit the problem):
class Meta(type):
def __getattribute__(self, attr):
if attr == "__bases__":
class Base: pass
return (Base, )
return type.__getattribute__(self, attr)
class X(metaclass=Meta):
pass
issubclass(X().__class__, int)
In this case, the refcount is decremented from 4 to 3 as abstract_issubclass()
gets rid of the tuple (instead of from 1 to 0 as happens in the rpyc case). I
don't know how rpyc does it.
Attached is a patch that solves the problem (takes a ref of the tuple item
before releasing the ref of the tuple itself).
I'm not sure this change is worth the cost because, well, I don't fully
understand the severity of it since I couldn't reproduce it without using rpyc.
I assume dynamically-created, unreferenced `__bases__` tuples as I have here
are not so common.
Anyway, if you do decide it's worth it, I'd be happy to improve the patch (it's
quite messy the way this function is structured) and post it to GitHub :)
Yonatan
----------
components: Interpreter Core
files: abstract_issubclass_refcount_fix.diff
keywords: patch
messages: 360247
nosy: Yonatan Goldschmidt
priority: normal
severity: normal
status: open
title: abstract_issubclass() doesn't take bases tuple item ref
type: crash
versions: Python 3.9
Added file:
https://bugs.python.org/file48851/abstract_issubclass_refcount_fix.diff
_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue39382>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
