classification
Title: Crash with custom __getattribute__
Type: crash Stage: needs patch
Components: Interpreter Core Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Trundle, belopolsky, daniel.urban, eric.araujo, flox, haypo, meador.inge, python-dev
Priority: normal Keywords: patch

Created on 2010-09-03 12:13 by flox, last changed 2011-05-01 21:52 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
issue9756.patch Trundle, 2011-04-29 16:01 review
Messages (18)
msg115446 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2010-09-03 12:13
I found this crash while playing with proxies (thanks haypo).
http://code.activestate.com/recipes/496741-object-proxying/


class MyClass(object):

    def __init__(self):
        self.pwn = None

    def __getattribute__(self, name):
        print('MyClass.__getattribute__(self, %r)' % name)
        return getattr('abc', name)

instance = MyClass()
str.strip(instance)
msg115447 - (view) Author: Andreas Stührk (Trundle) Date: 2010-09-03 12:33
It's because you can fool `PyObject_IsInstance()` that way:

>>> class Spam(object):
...     def __getattribute__(self, name):
...         if name == '__class__':
...             return str
...         raise AttributeError
... 
>>> isinstance(Spam(), str)
True
msg115472 - (view) Author: Andreas Stührk (Trundle) Date: 2010-09-03 17:20
At least two tests in `test.test_descr` consider that behaviour as a feature: "test_isinst_isclass" and "test_proxy_super".
msg115485 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-09-03 19:27
<< I found this crash while playing with proxies (thanks haypo).
http://code.activestate.com/recipes/496741-object-proxying/ >>

My question was: why does isinstance(Proxy('abc'), str) works (give True), whereas re.match('abc', Proxy('abc')) fail.

It looks like you gave me the answer ;-)
msg115515 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-09-03 22:08
>>> class Spam(object):
...     def __getattribute__(self, name):
...         if name == '__class__':
...             return str
...         raise AttributeError
... 
>>> spam = Spam('spam')
>>> isinstance(spam, str)
True

isinstance(spam, str) calls str.__instancecheck__(spam) (type___instancecheck__) which checks spam.__class__, and spam.__class__ is str.

--

>>> import re
>>> re.match('spam', spam)
TypeError: expected string or buffer

Here the result is different because _sre.compile(spam) calls getstring() which uses PyUnicode_Check(spam) (-> False) and does checks on the buffer API (-> False).

--

About str.title(spam): it calls methoddescr_call() which checks the type using... PyObject_IsInstance() (and not PyUnicode_Check()).
msg115550 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-09-04 00:33
PyUnicode_Check(op) checks op->ob_type->tp_flags & Py_TPFLAGS_UNICODE_SUBCLASS.
msg115552 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-09-04 00:38
I have different questions:
 - Should we trust PyObject_IsInstance() or PyUnicode_Check() (because they give different results)?
 - Should PyObject_IsInstance() and PyUnicode_Check() give the same result?
 - Should we fix the segfault?

To fix the segfault, I suppose that we use a more strict validation on the input type. Eg. Use PyUnicode_Check() instead of PyObject_IsInstance()?
msg115671 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2010-09-05 20:36
> To fix the segfault, I suppose that we use a more strict validation on 
> the input type. Eg. Use PyUnicode_Check() instead of 
> PyObject_IsInstance()?

Where would the more strict validation take place?  This problem is not unique to Unicode objects:

motherbrain:py3k minge$ ./python.exe 
Python 3.2a2 (py3k:84541, Sep  5 2010, 15:11:19) 
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class MyClass(object):
...    def __init__(self):
...       self.pwn = None
...    def __getattribute__(self, name):
...       return getattr(187, name)
... 
[49633 refs]
>>> instance = MyClass()
[49640 refs]
>>> int.bit_length(instance)
Assertion failed: (PyLong_Check(v)), function long_bit_length, file Objects/longobject.c, line 4397.
Abort trap
msg127869 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-02-04 04:54
Here is a somewhat more straightforward way to reproduce the problem:

>>> class X:
...    __class__ = int
... 
[55910 refs]
>>> isinstance(X(), int)
True
[55914 refs]
>>> int.bit_length(X())
Assertion failed: (PyLong_Check(v)), function long_bit_length, file ../py3k-commit/Objects/longobject.c, line 4413.
Abort trap

The place to implement a stricter check would be in methoddescr_call() function defined in Objects/descrobject.c, but it may affect legitimate tricks.  On the other hand, requiring that every C implemented method checks the type of self in a type-specific way is probably not feasible either.
msg127886 - (view) Author: Andreas Stührk (Trundle) Date: 2011-02-04 13:21
See also issue #10922.
msg134776 - (view) Author: Andreas Stührk (Trundle) Date: 2011-04-29 16:01
I think it is reasonable to restrict the self argument of method descriptors and slot wrapper descriptors to real instances of the type. The called method can't cope with the value anyway (in the general case). Alternative Python implementations like Jython and PyPy already enforce this.

Attached is a patch against default branch that enforces this.
msg134898 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-01 03:19
The test suite crashs randomly with issue9756.patch on my Ubuntu 11.04 (AMD64 with 4 cores, 4 GB of memory, Linux 2.6.38). I use "./python -bb Lib/test/regrtest.py -r" to reproduce the crash.

... I tried without the patch, and test_descr does crash quickly in test_wrapper_segfault(). It crashs in _PyObject_GC_New(), in a callback used to release the memory. Sometimes it fails on update_refs(), sometimes in _PyObject_DebugFree(), sometimes in wrapper_dealloc()->_Py_ForgetReference, etc. I use "./python Lib/test/regrtest.py -F -v test_descr" to reproduce the crash (sometimes it takes more than one run to crash).

It's strange because I'm unable to reproduce the bug on a very similar setup (Debian Sid, AMD64 with 2 cores, 2 GB of memory, Linux 2.6.38).

Should I run a memcheck on the Ubuntu host? Or is anyone able to reproduce the bug?
msg134899 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-01 04:00
Oh, the Ubuntu host has a temperature issue and hardware errors... Example of mcelog output:
--------------------------------
MCE 0
CPU 0 THERMAL EVENT TSC 198d1eb8325
TIME 1304222195 Sun May  1 05:56:35 2011
Processor 0 heated above trip temperature. Throttling enabled.
Please check your system cooling. Performance will be impacted
STATUS 8801000f MCGSTATUS 0
MCGCAP 806 APICID 0 SOCKETID 0
CPUID Vendor Intel Family 6 Model 23
HARDWARE ERROR. This is *NOT* a software problem!
Please contact your hardware vendor
--------------------------------
msg134906 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-01 13:09
Le dimanche 01 mai 2011 à 03:19 +0000, STINNER Victor a écrit :
> The test suite crashs randomly with issue9756.patch on my Ubuntu 11.04
> (AMD64 with 4 cores, 4 GB of memory, Linux 2.6.38). I use "./python
> -bb Lib/test/regrtest.py -r" to reproduce the crash.
> 
> ... I tried without the patch, and test_descr does crash quickly in
> test_wrapper_segfault(). It crashs in _PyObject_GC_New(), in a
> callback used to release the memory. Sometimes it fails on
> update_refs(), sometimes in _PyObject_DebugFree(), sometimes in
> wrapper_dealloc()->_Py_ForgetReference, etc. I use "./python
> Lib/test/regrtest.py -F -v test_descr" to reproduce the crash
> (sometimes it takes more than one run to crash).
> 
> It's strange because I'm unable to reproduce the bug on a very similar
> setup (Debian Sid, AMD64 with 2 cores, 2 GB of memory, Linux 2.6.38).
> 
> Should I run a memcheck on the Ubuntu host? Or is anyone able to
> reproduce the bug?

Ok, I'm sure that I have hardware issues, but I am also sure that the
patch introduces random crashes, especially in
test_descr.test_wrapper_segfault().

I tried to only patch methoddescr_call(): replace PyObject_IsInstance()
by _PyObject_RealIsSubclass() introduces the crash. Call
PyObject_IsInstance() after _PyObject_RealIsSubclass() doesn't help.
But... call PyObject_IsInstance() before _PyObject_RealIsSubclass()
avoids the crash !?
msg134929 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-01 21:33
New changeset c5e6f997730e by Victor Stinner in branch '3.1':
Issue #9756: When calling a method descriptor or a slot wrapper descriptor, the
http://hg.python.org/cpython/rev/c5e6f997730e

New changeset 4fc04f6a0731 by Victor Stinner in branch '3.2':
(Merge 3.1) Issue #9756: When calling a method descriptor or a slot wrapper
http://hg.python.org/cpython/rev/4fc04f6a0731

New changeset 8544561b7f04 by Victor Stinner in branch 'default':
(Merge 3.2) Issue #9756: When calling a method descriptor or a slot wrapper
http://hg.python.org/cpython/rev/8544561b7f04
msg134930 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-01 21:44
New changeset 109687cc2c1e by Victor Stinner in branch '2.7':
(Merge 3.1) Issue #9756: When calling a method descriptor or a slot wrapper
http://hg.python.org/cpython/rev/109687cc2c1e
msg134931 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-01 21:51
New changeset 0db11682ea45 by Victor Stinner in branch '3.1':
Issue #9756: credit the author, Andreas Stührk (Trundle)
http://hg.python.org/cpython/rev/0db11682ea45

New changeset a17f5c787cc0 by Victor Stinner in branch '3.2':
(Merge 3.1) Issue #9756: credit the author, Andreas Stührk (Trundle)
http://hg.python.org/cpython/rev/a17f5c787cc0

New changeset ba8a8c47de7b by Victor Stinner in branch 'default':
(Merge 3.2) Issue #9756: credit the author, Andreas Stührk (Trundle)
http://hg.python.org/cpython/rev/ba8a8c47de7b

New changeset bc5cd43c8d0c by Victor Stinner in branch '2.7':
(Merge 3.1) Issue #9756: credit the author, Andreas Stührk (Trundle)
http://hg.python.org/cpython/rev/bc5cd43c8d0c
msg134932 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-05-01 21:52
> Ok, I'm sure that I have hardware issues, but I am also sure that the
> patch introduces random crashes, especially in
> test_descr.test_wrapper_segfault().

Forget my last messages: my Ubuntu box has serious memory issues. memtest86+ found 41 errors.

--

I applied Trundle's patch to 2.7, 3.1, 3.2 and 3.3, thanks for the fix.
History
Date User Action Args
2011-05-01 23:09:24haypolinkissue10922 superseder
2011-05-01 21:52:28hayposetstatus: open -> closed
resolution: fixed
messages: + msg134932
2011-05-01 21:51:25python-devsetmessages: + msg134931
2011-05-01 21:44:32python-devsetmessages: + msg134930
2011-05-01 21:33:17python-devsetnosy: + python-dev
messages: + msg134929
2011-05-01 13:09:06hayposetmessages: + msg134906
2011-05-01 04:00:27hayposetmessages: + msg134899
2011-05-01 03:19:05hayposetmessages: + msg134898
2011-04-29 16:01:10Trundlesetfiles: + issue9756.patch
keywords: + patch
messages: + msg134776
2011-02-04 13:21:38Trundlesetnosy: belopolsky, haypo, eric.araujo, Trundle, flox, meador.inge, daniel.urban
messages: + msg127886
2011-02-04 04:54:18belopolskysetnosy: + belopolsky
messages: + msg127869
2010-09-15 19:31:41daniel.urbansetnosy: + daniel.urban
2010-09-12 00:18:18eric.araujosetnosy: + eric.araujo

versions: - Python 2.6, Python 2.5
2010-09-05 20:36:37meador.ingesetnosy: + meador.inge
messages: + msg115671
2010-09-04 00:38:52hayposetmessages: + msg115552
2010-09-04 00:33:03hayposetmessages: + msg115550
2010-09-03 22:08:25hayposetmessages: + msg115515
2010-09-03 19:27:20hayposetmessages: + msg115485
2010-09-03 17:20:58Trundlesetmessages: + msg115472
2010-09-03 12:33:14Trundlesetnosy: + Trundle
messages: + msg115447
2010-09-03 12:13:02floxcreate