[dm-devel] [RFC][PATCH] Add checks for correct hardsect boundary alignment

[Christophe Saout]

Hi,

I think we should add additional checks to ensure that we don't run
into trouble with devices that have a non-512-byte sector size.

In particular we need to check that dm_get_device is called with
a matching begin sector number and sector length.

"Matching" meaning that the given sectors are a multiple of
hardsect_size >> SECTOR_SHIFT.

This also counts for split_io.

There is one issue that I'm not sure about: If a target with
hardsect_size > 512 is on an "uneven" tgt->begin __clone_and_map might
split a correctly sized bio into incorrectly sized ones.

So probably tgt->begin and tgt->len should be checked the same way
as split_io. Hmmm.


Add checks for correct hardsect boundary alignment.
--- linux.orig/drivers/md/dm-table.c	2004-01-20 16:31:14.627515048 +0100
+++ linux/drivers/md/dm-table.c	2004-01-20 16:51:47.963019552 +0100
@@ -389,6 +389,17 @@
 }
 
 /*
+ * Checks if the given sector count is compatible with the
+ * hardware sector size restriction of the target.
+ */
+static int check_hardsect_boundary(struct io_restrictions *rs,
+                                   sector_t sectors)
+{
+	int sector_size = rs->hardsect_size >> SECTOR_SHIFT;
+	return !(sectors & (sector_size - 1));
+}
+
+/*
  * This upgrades the mode on an already open dm_dev.  Being
  * careful to leave things as they were if we fail to reopen the
  * device.
@@ -496,6 +507,10 @@
 			.seg_boundary_mask = q->seg_boundary_mask
 		};
 
+		if (!check_hardsect_boundary(&hrs, start) ||
+		    !check_hardsect_boundary(&hrs, len))
+			return -EINVAL;
+
 		/* FIXME: Device-Mapper on top of RAID-0 breaks because DM
 		 *        currently doesn't honor MD's merge_bvec_fn routine.
 		 *        In this case, we'll force DM to use PAGE_SIZE or
@@ -682,6 +697,15 @@
 	if (r)
 		goto bad;
 
+	/*
+	 * FIXME: Should we also check tgt->begin and tgt->len?
+	 */
+	if (!check_hardsect_boundary(&tgt->limits, tgt->split_io)) {
+		tgt->error = "hardsect boundaries disrespected";
+		r = -EINVAL;
+		goto bad;
+	}
+
 	t->highs[t->num_targets++] = tgt->begin + tgt->len - 1;
 
 	/* FIXME: the plan is to combine high here and then have