From: Mikulas Patocka <mpatocka@redhat.com>
Date: Wed, 2 Dec 2009 05:26:53 -0500
Subject: [md] lock snapshot while reading status
Message-id: <Pine.LNX.4.64.0912020023020.11782@hs20-bc2-1.build.redhat.com>
Patchwork-id: 21642
O-Subject: [RHEL5.5 PATCH] bz543307: lock snapshot while reading status
Bugzilla: 543307
RH-Acked-by: Alasdair G Kergon <agk@redhat.com>

Hi

This patch fixes a race condition --- used chunk count is being read
without holding the lock, thus invalid intermediate value can be read.

Upstream status: 2.6.32-rc6, 4c6fff445d7aa753957856278d4d93bcad6e2c14

Unlike in upstream code, only STATUSTYPE_INFO branch is locked. It is
because:
- STATUSTYPE_TABLE is static and doesn't change (so it's ok to not lock
it)
- userspace asks for STATUSTYPE_TABLE while suspended, creating a possible
race
A patch to lock only STATUSTYPE_INFO and not lock STATUSTYPE_TABLE is
already pending upstream acceptance.

Testing: the race is nonreproducible (it was found during code review), so
I only tested basic snapshot functionality with this patch applied.

Mikulas


diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 0c58db2..b03941a 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1154,6 +1154,7 @@ static int snapshot_status(struct dm_target *ti, status_type_t type,
 
 	switch (type) {
 	case STATUSTYPE_INFO:
+		down_write(&snap->lock);
 		if (!snap->valid)
 			snprintf(result, maxlen, "Invalid");
 		else {
@@ -1169,6 +1170,7 @@ static int snapshot_status(struct dm_target *ti, status_type_t type,
 			else
 				snprintf(result, maxlen, "Unknown");
 		}
+		up_write(&snap->lock);
 		break;
 
 	case STATUSTYPE_TABLE: