Hello there. in function_call() in funcobject.c, we have this comment: /* XXX This is broken if the caller deletes dict items! */ Now, I wonder what specifically is meant here? are we really talking about the 'callee' here? In PyEval_EvalCodeEx() it looks as though all keywords are always INCREFed, so the callee never gets a borrowed reference to something from the keyword dict. Maybe this comment is out of date, or can someone demonstrate how to break the code accordingly? The reason I ask is that I am debugging a really tricky crash case on our live servers and I am currently led to believe that the temporary array for the keyword dict is being overwritten somehow. Cheers, Kristján, CCP games.
I think we really *are* talking about the caller -- the caller owns
the dict, if it managed to delete something from the dict before the
callee can incref it, you'd have trouble. I don't immediately see how
this could happen, which is probably why I left it as an XXX
comment...
--Guido
On Feb 5, 2008 6:58 AM, Kristján Valur Jónsson
Hello there.
in function_call() in funcobject.c, we have this comment:
/* XXX This is broken if the caller deletes dict items! */
Now, I wonder what specifically is meant here? are we really talking about the ‚callee' here?
In PyEval_EvalCodeEx() it looks as though all keywords are always INCREFed, so the callee never gets a borrowed reference to something from the keyword dict.
Maybe this comment is out of date, or can someone demonstrate how to break the code accordingly?
The reason I ask is that I am debugging a really tricky crash case on our live servers and I am currently led to believe that the temporary array for the keyword dict is being overwritten somehow.
Cheers,
Kristján,
CCP games.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/guido%40python.org
-- --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido van Rossum wrote:
I think we really *are* talking about the caller -- the caller owns the dict, if it managed to delete something from the dict before the callee can incref it, you'd have trouble. I don't immediately see how this could happen, which is probably why I left it as an XXX comment...
I found one way to call python code before the callee can incref the args: the __eq__ between variable names and the dict entries. The following snippet crashes the trunk version on win32: class Name(str): def __eq__(self, other): del d[self] return str.__eq__(self, other) def __hash__(self): return str.__hash__(self) d = {Name("a"):1, Name("b"):2} def f(a, b): print a,b f(**d) # Segfault There are several variants of this crasher; they all have more than one keyword argument, and keywords strings must override __eq__ or __hash__. I could not find any other way to execute python code in this area. -- Amaury Forgeot d'Arc
On Feb 5, 2008 2:07 PM, Amaury Forgeot d'Arc
Guido van Rossum wrote:
I think we really *are* talking about the caller -- the caller owns the dict, if it managed to delete something from the dict before the callee can incref it, you'd have trouble. I don't immediately see how this could happen, which is probably why I left it as an XXX comment...
I found one way to call python code before the callee can incref the args: the __eq__ between variable names and the dict entries. The following snippet crashes the trunk version on win32:
class Name(str): def __eq__(self, other): del d[self] return str.__eq__(self, other) def __hash__(self): return str.__hash__(self)
d = {Name("a"):1, Name("b"):2} def f(a, b): print a,b
f(**d) # Segfault
There are several variants of this crasher; they all have more than one keyword argument, and keywords strings must override __eq__ or __hash__. I could not find any other way to execute python code in this area.
Thanks Amaury! Do you think it would be sufficient to change the PyString_Check() call in PyEval_EvalCodeEx into a PyString_CheckExact() call? Or is the proper fix to incref the values going into the kw array and decref them upon exit? -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Guido van Rossum wrote:
Thanks Amaury! Do you think it would be sufficient to change the PyString_Check() call in PyEval_EvalCodeEx into a PyString_CheckExact() call?
This would prevent this "attack", but would remain fragile - future developments could allow execution of python code somewhere.
Or is the proper fix to incref the values going into the kw array and decref them upon exit?
Yet Another Kind Of Tuple... However this seems the correct thing to do. In addition, if we agree to restrict arguments names to str (and disallow subclasses), there are easy optimizations in PyEval_EvalCodeEx, somewhere around the "XXX slow" comment (!) -- Amaury Forgeot d'Arc
On Feb 5, 2008 4:02 PM, Amaury Forgeot d'Arc
Guido van Rossum wrote:
Thanks Amaury! Do you think it would be sufficient to change the PyString_Check() call in PyEval_EvalCodeEx into a PyString_CheckExact() call?
This would prevent this "attack", but would remain fragile - future developments could allow execution of python code somewhere.
Or is the proper fix to incref the values going into the kw array and decref them upon exit?
Yet Another Kind Of Tuple... However this seems the correct thing to do.
Agreed.
In addition, if we agree to restrict arguments names to str (and disallow subclasses), there are easy optimizations in PyEval_EvalCodeEx, somewhere around the "XXX slow" comment (!)
Do you think you have time to come up with a patch? If not, can you file a bug for this so we won't forget? -- --Guido van Rossum (home page: http://www.python.org/~guido/)
-----Original Message----- From: Amaury Forgeot d'Arc [mailto:amauryfa@gmail.com] Sent: Wednesday, February 06, 2008 00:02 To: Guido van Rossum Cc: Kristján Valur Jónsson; python-dev@python.org Subject: Re: [Python-Dev] XXX - in funcobject.c
Yet Another Kind Of Tuple... However this seems the correct thing to do.
In addition, if we agree to restrict arguments names to str (and disallow subclasses), there are easy optimizations in PyEval_EvalCodeEx, somewhere around the "XXX slow" comment (!)
Super. I think I'll do this myself and see if the crashes go away (even though I know that doesn't constitute a proof). Also, allow me to suggest that we preallocate stack space for, say, 10 kwargs, to avoid the malloc for the common cases. Kristján
On Feb 6, 2008 1:49 AM, Kristján Valur Jónsson
-----Original Message----- From: Amaury Forgeot d'Arc [mailto:amauryfa@gmail.com] Sent: Wednesday, February 06, 2008 00:02 To: Guido van Rossum Cc: Kristján Valur Jónsson; python-dev@python.org Subject: Re: [Python-Dev] XXX - in funcobject.c
Yet Another Kind Of Tuple... However this seems the correct thing to do.
In addition, if we agree to restrict arguments names to str (and disallow subclasses), there are easy optimizations in PyEval_EvalCodeEx, somewhere around the "XXX slow" comment (!)
Super. I think I'll do this myself and see if the crashes go away (even though I know that doesn't constitute a proof). Also, allow me to suggest that we preallocate stack space for, say, 10 kwargs, to avoid the malloc for the common cases.
Great idea! If you come up with a useful patch, can you attach it to the appropriate bug issue? (issues 2015 and 2016 on bugs.python.org) -- --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (3)
-
Amaury Forgeot d'Arc
-
Guido van Rossum
-
Kristján Valur Jónsson