| msg19382 - (view) |
Author: Eric M. Hopper (omnifarious) |
Date: 2003-12-11 03:13 |
| This works for the PowerPC Python compiled with gcc 3.3 on OS X using fink. I suspect it's broader based than that, but I don't have the ability to check properly. Here's how to make it segment fault: x = (basestring,) for i in xrange(0, 1000000): x = (x,) issubclass(str, x) At least, it segment faults at the interactive prompt this way. I don't know if it does when it's executed from a file. |
|
|
| msg19383 - (view) |
Author: Eric M. Hopper (omnifarious) |
Date: 2003-12-11 03:16 |
| Logged In: YES user_id=313 I forgot this: Python 2.3.2 (#1, Dec 4 2003, 09:13:58) [GCC 3.3 20030304 (Apple Computer, Inc. build 1493)] on darwin Type "help", "copyright", "credits" or "license" for more information. |
|
|
| msg19384 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2003-12-11 04:28 |
| Logged In: YES user_id=357491 If you look at Object/abstract.c (line 2119 or so) for 2.4 CVS you will notice that PyObject_IsSubclass goes into a 'for' loop for each item in the tuple passed in and calls PyObject_IsSubclass . Unfortunately it makes no check for whether the argument it is passing is a class itself or not. This allows it to keep making calls as long as the second argument is either a class or a tuple. This is what is leads to the stack being blown and the subsequent segfault. Obvious solution is to put in a check that the argument about to be passed is a class itself so as to not have such a deep call chain. But since ``help(issubclass)`` actually makes the above use legit (it says using a tuple as a second argument is equivalent as passing each item to issubclass which is what it is doing, albeit in a rather uncommon and pointless way), is it worth putting the check in? Since this is such an obvious mis-use, I say no. But if someone else on python-dev steps in and says otherwise I will patch it. |
|
|
| msg19385 - (view) |
Author: Eric M. Hopper (omnifarious) |
Date: 2003-12-11 17:54 |
| Logged In: YES user_id=313 Well, I think any case where the system segment faults unexpectedly is bad, regardless of how pathological it is. Personally, I think that issubclass should either have a recursion limit after which it throws an exception, or it shouldn't go into sub-tuples at all. The reason I made this test is that I read the description of the behavior of issublcass and found it rather strange, so I decided to push it to see how far it would go. |
|
|
| msg19386 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-12-14 01:08 |
| Logged In: YES user_id=31435 Yes, this needs to be fixed if it *can* be fixed without heroic effort or insane slowdown. Looks like it can be. Brett, the missing piece of your worldview here is that anywhere Python can be tricked into segfaulting is a kind of "security hole" -- it's not just mistakes we want to protect programmers from, we also want to bulletproof against hostile users, to the extent sanely possible. BTW, if issubclass() has this insecurity, I bet isinstance() does too (they were introduced & coded at the same time). |
|
|
| msg19387 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2003-12-14 20:55 |
| Logged In: YES user_id=357491 OK, consider my worldview fixed. =) I will add a check in the tuple unpacking 'for' loop to make sure it is only passing issubclass classes and not more tuples. Simple and shouldn't break very much code. Otherwise the code would have to keep a count and extra bookkeeping and it would get messy quickly. And I will take a look at isinstance, although this tuple feature was added in 2.3 for issubclass so it might not be an issue. And I will backport it. |
|
|
| msg19388 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-12-14 21:09 |
| Logged In: YES user_id=31435 I'm afraid that changing semantics needs to be run through the python-dev wringer first -- "shouldn't break very much code" isn't "shouldn't break any code". The *comments* in these functions make it appear that they never intended to support the OP's original code snippet, but the docs don't match. This leaves the intent a mystery, so it needs to be discussed. |
|
|
| msg19389 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2003-12-18 21:39 |
| Logged In: YES user_id=357491 I have appended a file that adds a basic test to make sure that when the items of a tuple are used to call isinstance or issubclass that only classes or types are used; everything else raises TypeError. Also tried to clarify the wording of the doc strings. Changed the docs for isinstance since it allowed nested tupling while issubclass' didn't. Have a look and if someone will sign off on it I will apply the patch and then start working on a 2.3 solution that doesn't break semantics. And I just realized I left out a Misc/NEWS in the patch; it will be there when it gets applied. |
|
|
| msg19390 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2003-12-18 21:48 |
| Logged In: YES user_id=31435 Oh, Brett, you're missing a chance for some fun here! A bug should always be assigned to the next person who should "do something" about it. Think of it as a hot potato. You should assign the bug to who you *want* to review this, and then sit back and watch the fun, as each person in turn tries to unload the potato onto someone else. That's one way to get a comforting illusion of activity here . |
|
|
| msg19391 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2003-12-19 05:24 |
| Logged In: YES user_id=357491 OK, Tim, start hopping. =) |
|
|
| msg19392 - (view) |
Author: Tim Peters (tim.peters) * |
Date: 2004-02-16 23:44 |
| Logged In: YES user_id=31435 Well, that wasn't as much fun for you as I hoped -- the report just sat there until I got a free holiday . Marked Accepted and back to you -- looks good! |
|
|
| msg19393 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2004-03-20 16:14 |
| Logged In: YES user_id=357491 Have a patch for Python 2.3 that limits the depth of the tuples to the recursion limit of the interpreter. That will definitely be used for 2.3 Question is which solution to use for 2.4 . |
|
|
| msg19394 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2004-03-20 23:02 |
| Logged In: YES user_id=357491 Fixed in both Python 2.3 and 2.4 so that the depth of the tuple cannot exceed the the recursion limit of the interpreter. |
|
|