[ python-Bugs-1303614 ] Bypassing dict readonlyness

Mon Feb 26 09:23:03 CET 2007

Bugs item #1303614, was opened at 2005-09-24 23:40
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1303614&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Interpreter Core
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Armin Rigo (arigo)
Assigned to: Nobody/Anonymous (nobody)
Summary: Bypassing __dict__ readonlyness

Initial Comment:
The __dict__ attribute of some objects is read-only,
e.g. for type objects.  However, there is a generic
way to still access and modify it (without hacking with
gc.get_referrents(), that is).  This can lead to
obscure crashes.  Attached is an example that shows
a potential "problem" involving putting strange keys
in the __dict__ of a type.

This is probably very minor anyway.  If we wanted to
fix this, we would need a __dict__ descriptor that
looks more cleverly at the object to which it is
applied.

BTW the first person who understand why the attached
program crashes gets a free coffee.

----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2007-02-26 08:23

Message:
Logged In: YES 
user_id=4771
Originator: YES

456?  Now that's interesting.  Not 579?

I'm not sure if I ever thought about what the
expected answer should be, but I'd say that
you are correct: 'x.__dict__' should be empty
in the end.  Actually I don't really see how
it manages *not* to be empty in IronPython;
looks like either a very minor detail or a
deeper bug...

----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2007-02-25 22:36

Message:
Logged In: YES 
user_id=31392
Originator: NO

I tried test67.py in IronPython.  It reports that x.hello has the value
456.  Is that the correct behavior?  It seems incorrect to me.  If the
__eq__ method is called, then the object should no longer have either the
Strange() or hello attributes.  They are cleared by reseting __dict__.  Is
that right?

----------------------------------------------------------------------

Comment By: Jeremy Hylton (jhylton)
Date: 2007-02-25 22:23

Message:
Logged In: YES 
user_id=31392
Originator: NO

I tried test67.py in IronPython.  It reports that x.hello has the value
456.  Is that the correct behavior?  It seems incorrect to me.  If the
__eq__ method is called, then the object should no longer have either the
Strange() or hello attributes.  They are cleared by reseting __dict__.  Is
that right?

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2006-06-30 07:05

Message:
Logged In: YES 
user_id=4771

Well, try making an "empty" class Foo(object): pass, and see
what magically shows up in Foo.__dict__.keys().  Here is the
__dict__ descriptor used for instances of Foo.  This
descriptor is connected to subtype_dict() and
subtype_setdict() in typeobject.c.

INCREF/DECREFs are in theory missing around every use of the
dictionary returned by _PyObject_GetDictPtr(), because more
or less any such use could remove the dict from the object
from which _PyObject_GetDictPtr() returned from, and so
decref the dict - while it's used.  This might require some
timing, to check the performance impact.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2006-06-30 01:14

Message:
Logged In: YES 
user_id=357491

OK, then I am going to need a little guidance to dive into
this more since this is going into some murky waters for me.  =)

For the normal, non-evil case of an empty Python class
(``class Foo(object): pass``), I would think that accessing
__dict__ would fall through to the tp_getset for object, and
then fall to the same slot for type.  Now that is obviously
not happening since you get a straight dict back for that
attribute access and not a dict proxy as would be suggested
based on my logic listed above.

So, my question is, where is the code that handles fetching
Foo().__dict__?  And if you have an inkling of where I
should be looking in terms of possible refcounting additions
that would be great as well.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2006-06-29 21:19

Message:
Logged In: YES 
user_id=4771

Brett: I think you're approaching the problem from the wrong
angle.  The problem is being allowed to freely tamper with
the dict stored in objects.  Getting NULL errors here and
there is only a symptom.  As I explain, the '__dict__'
descriptor object needs to do some more checks, and to be
fully safe some Py_INCREF/Py_DECREF are needed in some
critical places.

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2006-06-29 17:45

Message:
Logged In: YES 
user_id=357491

For the deldict.py crasher, if you look at the traceback
there is no good place to do a check that tp_dict is sane
except in type_module() or PyDict_GetItem().  Now adding the
NULL check in type_module() will fix this specific problem,
but that seems like an ad-hoc patch.

Why don't we check for NULL in PyDict_GetItem() and return
NULL just like the PyDict_Check() test?  I am sure the
answer is "performance", but this is not PyDict_GETITEM()and
thus already is not the performance-critical version anyway.
 So why shouldn't we check for NULL there and possibly catch
other errors?

----------------------------------------------------------------------

Comment By: Brett Cannon (bcannon)
Date: 2006-06-29 17:41

Message:
Logged In: YES 
user_id=357491

Simple patch for the loosing_dict_ref.py crasher is
attached.  Just checked for possible NULL values in what is
needed by _Py_ForgetReference().  Let me know what you
think, Armin.

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2005-12-14 15:36

Message:
Logged In: YES 
user_id=6656

Yikes!

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2005-12-14 15:32

Message:
Logged In: YES 
user_id=4771

A proposed fix for the later crash: the __dict__ descriptor of
user-defined classes should verify if there is another __dict__ descriptor
along the solid path of the type (i.e. following "tp_base" pointers).  If
so, it should delegate to it.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2005-12-14 14:49

Message:
Logged In: YES 
user_id=4771

Uh oh.  There is a much simpler crash.  The technique outlined in
deldict.py can be used to rebind or even delete the __dict__ attribute of
instances where it should normally not be allowed.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2005-09-25 08:55

Message:
Logged In: YES 
user_id=4771

The bug is related to code like PyObject_GenericGetAttribute and
_PyType_Lookup which are not careful about the reference counters of the
objects they operate on.  This allows a much simpler crash (test67.py): the
__dict__ of an object can be decrefed away while lookdict() is looking at
it.  This has a simple fix -- drop some amount of Py_INCREF/Py_DECREF in
core routines like PyObject_GenericGetAttr.  We probably need to measure if
it has a performance impact, though.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1303614&group_id=5470

[ python-Bugs-1303614 ] Bypassing __dict__ readonlyness

[ python-Bugs-1303614 ] Bypassing dict readonlyness