public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Why aren't partitions limited to fit within the device?
@ 2006-10-12 23:50 Neil Brown
  2006-10-13  1:02 ` Andries Brouwer
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Neil Brown @ 2006-10-12 23:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: aeb, Jens Axboe


Hi,
 I was looking into an issue that someone was having with raid5.
They made an md/raid5 out of 5 whole devices and by luck the data
that was written to the first block of the 5th device looked
slightly like a partition table.  fdisk output below for the curious.
However some partitions were beyond the end of the device.

This resulted in annoying error messages popping up when various
programs such as mount (mounting by label) looked at various
partitions.
The messages were like:
Buffer I/O error on device sde3, logical block 1794
This logical block is way beyond the end of sde.

What is happening is that the partition is found to be beyond the end
of the device, a message is printed about this
   sde: p3 exceeds device capacity
but nothing is done about it.

So IO requests in the partition can go beyond the end of the device -
and the code that is supposed  to protect devices from this doesn't.
In ll_rw_blk.c, in generic_make_request,  the max bi_sector is checked
against maxsector and an error is reported if bi_sector is too large.
However this only happens before blk_partition_remap is called to map
the partition-sector to the device-sector.  The result of
blk_partition_remap is clearly expected to still be in range
	 *
	 * NOTE: we don't repeat the blk_size check for each new device.
	 * Stacking drivers are expected to know what they are doing.

Partitioning isn't exactly a 'stacking driver', but it doesn't seem
to know what it is doing :-)

So:  Is there any good reason to not clip the partitions to fit
within the device - and discard those that are completely beyond
the end of the device??

The patch at the end of the mail does that.  Is it OK to submit this
to mainline?

NeilBrown



# fdisk /dev/sde
Command (m for help): p

Disk /dev/sde: 250.0 GB, 250059350016 bytes
255 heads, 63 sectors/track, 30401 cylinders
Units = cylinders of 16065 * 512 = 8225280 bytes

   Device Boot      Start         End      Blocks   Id  System
/dev/sde1               1           1         995+  c7  Syrinx
Partition 1 does not end on cylinder boundary.
/dev/sde2               1           1           0    0  Empty
Partition 2 does not end on cylinder boundary.
/dev/sde3          133675      133675         995+  c7  Syrinx
Partition 3 does not end on cylinder boundary.
/dev/sde4               1           1           0    0  Empty
Partition 4 does not end on cylinder boundary.


Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./block/ioctl.c         |    6 ++++++
 ./fs/partitions/check.c |   12 ++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff .prev/block/ioctl.c ./block/ioctl.c
--- .prev/block/ioctl.c	2006-10-09 14:25:11.000000000 +1000
+++ ./block/ioctl.c	2006-10-13 09:45:52.000000000 +1000
@@ -42,6 +42,12 @@ static int blkpg_ioctl(struct block_devi
 				    || pstart < 0 || plength < 0)
 					return -EINVAL;
 			}
+			/* Does partition fit within device */
+			if (start + length > get_capacity(disk)) {
+				if (start >= get_capacity(disk))
+					return -EINVAL;
+				length = get_capacity(disk) - start;
+			}
 			/* partition number in use? */
 			mutex_lock(&bdev->bd_mutex);
 			if (disk->part[part - 1]) {

diff .prev/fs/partitions/check.c ./fs/partitions/check.c
--- .prev/fs/partitions/check.c	2006-10-13 09:28:21.000000000 +1000
+++ ./fs/partitions/check.c	2006-10-13 09:46:12.000000000 +1000
@@ -466,8 +466,16 @@ int rescan_partitions(struct gendisk *di
 		if (!size)
 			continue;
 		if (from + size > get_capacity(disk)) {
-			printk(" %s: p%d exceeds device capacity\n",
-				disk->disk_name, p);
+			if (from >= get_capacity(disk)) {
+				printk(KERN_INFO
+				       " %s: p%d starts beyond end of device\n",
+				       disk->disk_name, p);
+				continue;
+			}
+			printk(KERN_INFO
+			       " %s: p%d exceeds device capacity - clipping\n",
+			       disk->disk_name, p);
+			size = get_capacity(disk) - from;
 		}
 		add_partition(disk, p, from, size);
 #ifdef CONFIG_BLK_DEV_MD

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2006-10-17  6:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-12 23:50 Why aren't partitions limited to fit within the device? Neil Brown
2006-10-13  1:02 ` Andries Brouwer
2006-10-13  1:31   ` Neil Brown
2006-10-13 15:07 ` Alan Cox
2006-10-13 18:16   ` jdow
2006-10-16  0:08   ` Neil Brown
2006-10-16  1:22     ` Wakko Warner
2006-10-16  1:31     ` Randy Dunlap
2006-10-16  4:09     ` Kyle Moffett
2006-10-16  6:40     ` Andries Brouwer
2006-10-16  6:43       ` dean gaudet
2006-10-16  7:28     ` Xavier Bestel
2006-10-16 10:54       ` Alan Cox
2006-10-17  6:05         ` Chris Wedgwood
2006-10-15  8:29 ` Ville Herva
2006-10-15 23:50   ` Neil Brown
2006-10-16  5:27     ` Ville Herva
2006-10-16  6:02     ` Andries Brouwer
2006-10-16  6:20       ` Ville Herva
2006-10-16  6:27       ` dean gaudet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox