Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 4 Jul 2022 13:07:18 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: de0b1239dfa4 - stable/13 - vm: Fix racy checks for swap objects
Message-ID:  <202207041307.264D7Io1056733@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=de0b1239dfa49011f76369551838d31c00e5daa1

commit de0b1239dfa49011f76369551838d31c00e5daa1
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-06-20 16:18:15 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-07-04 13:06:55 +0000

    vm: Fix racy checks for swap objects
    
    Commit 4b8365d752ef introduced the ability to dynamically register
    VM object types, for use by tmpfs, which creates swap-backed objects.
    As a part of this, checks for such objects changed from
    
      object->type == OBJT_DEFAULT || object->type == OBJT_SWAP
    
    to
    
      object->type == OBJT_DEFAULT || (object->flags & OBJ_SWAP) != 0
    
    In particular, objects of type OBJT_DEFAULT do not have OBJ_SWAP set;
    the swap pager sets this flag when converting from OBJT_DEFAULT to
    OBJT_SWAP.
    
    A few of these checks are done without the object lock held.  It turns
    out that this can result in false negatives since the swap pager
    converts objects like so:
    
      object->type = OBJT_SWAP;
      object->flags |= OBJ_SWAP;
    
    Fix the problem by adding explicit tests for OBJT_SWAP objects in
    unlocked checks.
    
    PR:             258932
    Fixes:          4b8365d752ef ("Add OBJT_SWAP_TMPFS pager")
    Reported by:    bdrewery
    Reviewed by:    kib
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit e123264e4dc394602f9fed2f0376204b5998d815)
---
 sys/vm/vm_map.c     | 9 ++++++---
 sys/vm/vm_mmap.c    | 5 ++---
 sys/vm/vm_pageout.c | 5 +++--
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c
index fb59c2351a5d..7373bb21705a 100644
--- a/sys/vm/vm_map.c
+++ b/sys/vm/vm_map.c
@@ -2826,9 +2826,6 @@ again:
 			continue;
 		}
 
-		if (obj->type != OBJT_DEFAULT &&
-		    (obj->flags & OBJ_SWAP) == 0)
-			continue;
 		VM_OBJECT_WLOCK(obj);
 		if (obj->type != OBJT_DEFAULT &&
 		    (obj->flags & OBJ_SWAP) == 0) {
@@ -4141,7 +4138,13 @@ vm_map_copy_entry(
 		 */
 		size = src_entry->end - src_entry->start;
 		if ((src_object = src_entry->object.vm_object) != NULL) {
+			/*
+			 * Swap-backed objects need special handling.  Note that
+			 * this is an unlocked check, so it is possible to race
+			 * with an OBJT_DEFAULT -> OBJT_SWAP conversion.
+			 */
 			if (src_object->type == OBJT_DEFAULT ||
+			    src_object->type == OBJT_SWAP ||
 			    (src_object->flags & OBJ_SWAP) != 0) {
 				vm_map_copy_swap_object(src_entry, dst_entry,
 				    size, fork_charge);
diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c
index adbecd01ef38..48bf364731a2 100644
--- a/sys/vm/vm_mmap.c
+++ b/sys/vm/vm_mmap.c
@@ -1356,9 +1356,8 @@ vm_mmap_vnode(struct thread *td, vm_size_t objsize,
 			goto done;
 		}
 	} else {
-		KASSERT(obj->type == OBJT_DEFAULT ||
-		    (obj->flags & OBJ_SWAP) != 0,
-		    ("wrong object type"));
+		KASSERT(obj->type == OBJT_DEFAULT || obj->type == OBJT_SWAP ||
+		    (obj->flags & OBJ_SWAP) != 0, ("wrong object type"));
 		vm_object_reference(obj);
 #if VM_NRESERVLEVEL > 0
 		if ((obj->flags & OBJ_COLORED) == 0) {
diff --git a/sys/vm/vm_pageout.c b/sys/vm/vm_pageout.c
index 36d5f3275800..74439d5884ef 100644
--- a/sys/vm/vm_pageout.c
+++ b/sys/vm/vm_pageout.c
@@ -1886,8 +1886,9 @@ vm_pageout_oom_pagecount(struct vmspace *vmspace)
 		if ((entry->eflags & MAP_ENTRY_NEEDS_COPY) != 0 &&
 		    obj->ref_count != 1)
 			continue;
-		if (obj->type == OBJT_DEFAULT || obj->type == OBJT_PHYS ||
-		    obj->type == OBJT_VNODE || (obj->flags & OBJ_SWAP) != 0)
+		if (obj->type == OBJT_DEFAULT || obj->type == OBJT_SWAP ||
+		    obj->type == OBJT_PHYS || obj->type == OBJT_VNODE ||
+		    (obj->flags & OBJ_SWAP) != 0)
 			res += obj->resident_page_count;
 	}
 	return (res);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202207041307.264D7Io1056733>