From: Jeremy Hylton <jeremy@alum.mit.edu>
Date: Thu, 3 Apr 2003 23:02:29 +0000 (+0000)
Subject: Fix memory corruption in garbage collection.
X-Git-Tag: v2.2.3c1~85
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f95350079cb0e93c57709b1166f969505dfd33a1;p=thirdparty%2FPython%2Fcpython.git

Fix memory corruption in garbage collection.

The move_finalizers() routine checks each object in the unreachable
list to see if it has a finalizer.  If it does, it is moved to the
finalizers list.  The collector checks by calling, effectively,
hasattr(obj, "__del__").  The hasattr() call can result in an
arbitrary amount of Python code being run, because it will invoke
getattr hooks on obj.

If a getattr() hook is run from move_finalizers(), it may end up
resurrecting or deallocating objects in the unreachable list.  In
fact, it's possible that the hook causes the next object in the list
to be deallocated.  That is, the object pointed to by gc->gc.gc_next
may be freed before has_finalizer() returns.

The problem with the previous revision is that it followed
gc->gc.gc_next before calling has_finalizer().  If has_finalizer()
gc->happened to deallocate the object FROM_GC(gc->gc.gc_next), then
the next time through the loop gc would point to freed memory.  The
fix is to always follow the next pointer after calling
has_finalizer().

Note that Python 2.3 does not have this problem, because
has_finalizer() checks the tp_del slot and never runs Python code.

Tim, Barry, and I peed away the better part of two days tracking this
down.
---

diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index 85baa2fc1c4c..4dd159033328 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -276,12 +276,16 @@ move_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers)
 	PyGC_Head *gc = unreachable->gc.gc_next;
 	for (; gc != unreachable; gc=next) {
 		PyObject *op = FROM_GC(gc);
-		next = gc->gc.gc_next;
+		/* has_finalizer() may result in arbitrary Python
+		   code being run. */
 		if (has_finalizer(op)) {
+			next = gc->gc.gc_next;
 			gc_list_remove(gc);
 			gc_list_append(gc, finalizers);
 			gc->gc.gc_refs = GC_MOVED;
 		}
+		else
+			next = gc->gc.gc_next;
 	}
 }