This would be good for 2.6.19 and even 18.2, if it is seens acceptable.
raid0 at least (possibly other) can be made to Oops with a bad partition
table and best fix seem to be to not let out-of-range request get down
to the device.
### Comments for Changeset
Partitions are not limited to live within a device. So
we should range check after partition mapping.
Note that 'maxsector' was being used for two different things. I have
split off the second usage into 'old_sector' so that maxsector can be
still be used for it's primary usage later in the function.
Cc: Jens Axboe <[email protected]>
Signed-off-by: Neil Brown <[email protected]>
### Diffstat output
./block/ll_rw_blk.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff .prev/block/ll_rw_blk.c ./block/ll_rw_blk.c
--- .prev/block/ll_rw_blk.c 2006-10-31 11:43:33.000000000 +1100
+++ ./block/ll_rw_blk.c 2006-10-31 12:47:07.000000000 +1100
@@ -3007,6 +3007,7 @@ static inline void __generic_make_reques
{
request_queue_t *q;
sector_t maxsector;
+ sector_t old_sector;
int ret, nr_sectors = bio_sectors(bio);
dev_t old_dev;
@@ -3035,7 +3036,7 @@ static inline void __generic_make_reques
* NOTE: we don't repeat the blk_size check for each new device.
* Stacking drivers are expected to know what they are doing.
*/
- maxsector = -1;
+ old_sector = -1;
old_dev = 0;
do {
char b[BDEVNAME_SIZE];
@@ -3069,15 +3070,30 @@ end_io:
*/
blk_partition_remap(bio);
- if (maxsector != -1)
+ if (old_sector != -1)
blk_add_trace_remap(q, bio, old_dev, bio->bi_sector,
- maxsector);
+ old_sector);
blk_add_trace_bio(q, bio, BLK_TA_QUEUE);
- maxsector = bio->bi_sector;
+ old_sector = bio->bi_sector;
old_dev = bio->bi_bdev->bd_dev;
+ maxsector = bio->bi_bdev->bd_inode->i_size >> 9;
+ if (maxsector) {
+ sector_t sector = bio->bi_sector;
+
+ if (maxsector < nr_sectors || maxsector - nr_sectors < sector) {
+ /*
+ * This may well happen - partitions are not checked
+ * to make sure they are within the size of the
+ * whole device.
+ */
+ handle_bad_sector(bio);
+ goto end_io;
+ }
+ }
+
ret = q->make_request_fn(q, bio);
} while (ret);
}
On Tue, Oct 31 2006, NeilBrown wrote:
> This would be good for 2.6.19 and even 18.2, if it is seens acceptable.
> raid0 at least (possibly other) can be made to Oops with a bad partition
> table and best fix seem to be to not let out-of-range request get down
> to the device.
>
> ### Comments for Changeset
>
> Partitions are not limited to live within a device. So
> we should range check after partition mapping.
>
> Note that 'maxsector' was being used for two different things. I have
> split off the second usage into 'old_sector' so that maxsector can be
> still be used for it's primary usage later in the function.
>
> Cc: Jens Axboe <[email protected]>
> Signed-off-by: Neil Brown <[email protected]>
Code looks good to me, but for some reason your comment exceeds 80
chars. Can you please fix that up?
Acked-by: Jens Axboe <[email protected]>
--
Jens Axboe
* Jens Axboe ([email protected]) wrote:
> On Tue, Oct 31 2006, NeilBrown wrote:
> > This would be good for 2.6.19 and even 18.2, if it is seens acceptable.
> > raid0 at least (possibly other) can be made to Oops with a bad partition
> > table and best fix seem to be to not let out-of-range request get down
> > to the device.
> >
> > ### Comments for Changeset
> >
> > Partitions are not limited to live within a device. So
> > we should range check after partition mapping.
> >
> > Note that 'maxsector' was being used for two different things. I have
> > split off the second usage into 'old_sector' so that maxsector can be
> > still be used for it's primary usage later in the function.
> >
> > Cc: Jens Axboe <[email protected]>
> > Signed-off-by: Neil Brown <[email protected]>
>
> Code looks good to me, but for some reason your comment exceeds 80
> chars. Can you please fix that up?
Maybe just was copy, pasted and pushed one tab over from the same check
above the loop. What about consolidating that one?
thanks,
-chris
--
From: Neil Brown <[email protected]>
Partitions are not limited to live within a device. So
we should range check after partition mapping.
Note that 'maxsector' was being used for two different things. I have
split off the second usage into 'old_sector' so that maxsector can be
still be used for it's primary usage later in the function.
Acked-by: Jens Axboe <[email protected]>
Signed-off-by: Neil Brown <[email protected]>
Add check_bad_sector() to consolidate the checks a touch.
Signed-off-by: Chris Wright <[email protected]>
---
(smth like this only compile tested extension to Neil's patch)
block/ll_rw_blk.c | 51 +++++++++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 20 deletions(-)
--- a/block/ll_rw_blk.c
+++ b/block/ll_rw_blk.c
@@ -2971,6 +2971,26 @@ static void handle_bad_sector(struct bio
set_bit(BIO_EOF, &bio->bi_flags);
}
+static int check_bad_sector(struct bio *bio)
+{
+ sector_t maxsector = bio->bi_bdev->bd_inode->i_size >> 9;
+ int nr_sectors = bio_sectors(bio);
+ if (maxsector) {
+ sector_t sector = bio->bi_sector;
+
+ if (maxsector < nr_sectors || maxsector - nr_sectors < sector) {
+ /*
+ * This may well happen - the kernel calls bread()
+ * without checking the size of the device, e.g., when
+ * mounting a device.
+ */
+ handle_bad_sector(bio);
+ return 1;
+ }
+ }
+ return 0;
+}
+
/**
* generic_make_request: hand a buffer to its device driver for I/O
* @bio: The bio describing the location in memory and on the device.
@@ -2998,26 +3018,14 @@ static void handle_bad_sector(struct bio
void generic_make_request(struct bio *bio)
{
request_queue_t *q;
- sector_t maxsector;
- int ret, nr_sectors = bio_sectors(bio);
+ sector_t old_sector;
+ int ret;
dev_t old_dev;
might_sleep();
/* Test device or partition size, when known. */
- maxsector = bio->bi_bdev->bd_inode->i_size >> 9;
- if (maxsector) {
- sector_t sector = bio->bi_sector;
-
- if (maxsector < nr_sectors || maxsector - nr_sectors < sector) {
- /*
- * This may well happen - the kernel calls bread()
- * without checking the size of the device, e.g., when
- * mounting a device.
- */
- handle_bad_sector(bio);
- goto end_io;
- }
- }
+ if (check_bad_sector(bio))
+ goto end_io;
/*
* Resolve the mapping until finished. (drivers are
@@ -3027,7 +3035,7 @@ void generic_make_request(struct bio *bi
* NOTE: we don't repeat the blk_size check for each new device.
* Stacking drivers are expected to know what they are doing.
*/
- maxsector = -1;
+ old_sector = -1;
old_dev = 0;
do {
char b[BDEVNAME_SIZE];
@@ -3061,15 +3069,18 @@ end_io:
*/
blk_partition_remap(bio);
- if (maxsector != -1)
+ if (old_sector != -1)
blk_add_trace_remap(q, bio, old_dev, bio->bi_sector,
- maxsector);
+ old_sector);
blk_add_trace_bio(q, bio, BLK_TA_QUEUE);
- maxsector = bio->bi_sector;
+ old_sector = bio->bi_sector;
old_dev = bio->bi_bdev->bd_dev;
+ if (check_bad_sector(bio))
+ goto end_io;
+
ret = q->make_request_fn(q, bio);
} while (ret);
}