(From Alan):
authorMatthew Dillon <dillon@dragonflybsd.org>
Wed, 28 Jul 2004 20:40:35 +0000 (20:40 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Wed, 28 Jul 2004 20:40:35 +0000 (20:40 +0000)
 Correct a very old error in both vm_object_madvise() (originating in
 vm/vm_object.c revision 1.88) and vm_object_sync() (originating in
 vm/vm_map.c revision 1.36): When descending a chain of backing objects,
 both use the wrong object's backing offset.  Consequently, both may
 operate on the wrong pages.

(From Matt):
 In DragonFly the code needing correction is in vm_object_madvise() and
 vm_map_clean() (that code in vm_map_clean() was moved to vm_object_sync()
 in FreebSD-5 hence the FreeBSD-5 correction made by Alan was slight
 different).

 The madvise case could produce corrupted user memory when MADV_FREE was
 used, primarily on server-forked processes (where shadow objects exist)
 PLUS a special set of additional circumstances:  (1) The deeper shadow
 layers had to no longer be shared, (2) Either the memory had been swapped
 out in deeper shadow layers (not just the first shadow layer), resulting
 in the wrong swap space being freed, or (2) the forked memory had not yet
 been COW'd (and the deeper shadow layer is no longer shared) AND also had
 not yet been collapsed backed into the parent (e.g.  the original parent
 and/or other forks had exited and/or the memory had been isolated from
 them already).

 This bug could be responsible for all of the sporatic madvise oddness
 that has been reported over the years, especially in earlier days when
 systems had less memory and paged to swap a lot more then they do today.
 These weird failure cases have led people to generally not use MADV_FREE
 (in particular the 'H' malloc.conf option) as much as they could.  Also
 note that I tightened up the VM object collapse code considerably in
 FreeBSD-4.x making the failure cases above even less likely to occur.

 The vm_map_clean() (vm_object_sync() in FreeBSD-5) case is not likely
 to produce failures and it might not even be possible for it to occur
 in the first place since it requires PROT_WRITE mapped vnodes to exist
 in a backing object, which either might not be possible or might only occur
 under extrodinary circumstances.  Plus the worst that happens is that the
 vnode's data doesn't get written out immediately (but always will later on).

 Kudos to Alan for finding this old bug!

Noticed and corrected by: Alan Cox <alc@cs.rice.edu>
See also: FreeBSD vm_object.c/1.329

sys/vm/vm_map.c
sys/vm/vm_object.c

index 0ee74a7..9ddb75e 100644 (file)
@@ -62,7 +62,7 @@
  * rights to redistribute these changes.
  *
  * $FreeBSD: src/sys/vm/vm_map.c,v 1.187.2.19 2003/05/27 00:47:02 alc Exp $
- * $DragonFly: src/sys/vm/vm_map.c,v 1.30 2004/07/24 20:25:47 dillon Exp $
+ * $DragonFly: src/sys/vm/vm_map.c,v 1.31 2004/07/28 20:40:35 dillon Exp $
  */
 
 /*
@@ -2165,8 +2165,8 @@ vm_map_clean(vm_map_t map, vm_offset_t start, vm_offset_t end, boolean_t syncio,
                 * may start out with a NULL object.
                 */
                while (object && object->backing_object) {
-                       object = object->backing_object;
                        offset += object->backing_object_offset;
+                       object = object->backing_object;
                        if (object->size < OFF_TO_IDX( offset + size))
                                size = IDX_TO_OFF(object->size) - offset;
                }
index ab6e932..09bf51a 100644 (file)
@@ -62,7 +62,7 @@
  * rights to redistribute these changes.
  *
  * $FreeBSD: src/sys/vm/vm_object.c,v 1.171.2.8 2003/05/26 19:17:56 alc Exp $
- * $DragonFly: src/sys/vm/vm_object.c,v 1.16 2004/05/13 17:40:19 dillon Exp $
+ * $DragonFly: src/sys/vm/vm_object.c,v 1.17 2004/07/28 20:40:35 dillon Exp $
  */
 
 /*
@@ -1011,10 +1011,10 @@ shadowlookup:
                         * next object
                         */
                        splx(s);
-                       tobject = tobject->backing_object;
-                       if (tobject == NULL)
+                       if (tobject->backing_object == NULL)
                                continue;
                        tpindex += OFF_TO_IDX(tobject->backing_object_offset);
+                       tobject = tobject->backing_object;
                        goto shadowlookup;
                }