git: plan9front

Info  •  Files  •  Log  •  Branches

kernel: revert locking change for putseg(), it is A/B/A bug.

The previous attempt on avoiding having to acquire
Image lock introduces a A/B/A issue.

If we let Segment.ref drop to zero before removing
our segment from the Image cache, theres a race
where someone can acquire the Image lock first,
take a reference and then call putseg() again,
freeing the segment from under us. Once our
lock(i) succeeds, s is freed.

--- a/sys/src/9/port/segment.c

+++ b/sys/src/9/port/segment.c

@@ -94,13 +94,24 @@

 	Pte **pte, **emap;

 	Image *i;

-	if(s == nil || decref(s) != 0)

+	if(s == nil)

 		return;

 	i = s->image;

 	if(i != nil) {

+		/*

+		 *  We must hold image lock here during

+		 *  decref() to prevent someone from taking

+		 *  a reference to our segment from the cache.

+		 *  Just letting decref(s) drop to zero *before*

+		 *  and then checking s->ref again under image

+		 *  lock is not sufficient, as someone can grab

+		 *  a reference and then call putseg() again;

+		 *  freeing segment. By the time we hold image lock,

+		 *  the segment would be freed from under us.

+		 */

 		lock(i);

-		if(s->ref != 0){

+		if(decref(s) != 0){

 			unlock(i);

 			return;

 		}

@@ -107,7 +118,8 @@

 		if(i->s == s)

 			i->s = nil;

 		putimage(i);

-	}

+	} else if(decref(s) != 0)

+		return;

 	assert(s->sema.prev == &s->sema);

 	assert(s->sema.next == &s->sema);

-- 

⑨