Skip to contentSkip to conversation

block: Fix bad range check in bio_sector_offset

PATCH In review Martin K. Petersen 3 participants 5 messages Aug 28, 2012 – Aug 29, 2012 VFS

lore
Share & tools
lore

Martin K. Petersen

author
#0 lore raw

PATCH 1/1 block: Fix bad range check in bio_sector_offset

DM would occasionally end up splitting data integrity-enabled requests incorrectly. The culprit was a bad range check in bio_sector_offset.

Signed-off-by Martin K. Petersen <[email protected]>
Cc <[email protected]>
fs/bio.c +2 -2
index 9bfade8..b9a6744 100644
@@ -1552,8 +1552,8 @@ sector_t bio_sector_offset(struct bio *bio, unsigned short index,
 	sector_sz = queue_logical_block_size(bio->bi_bdev->bd_disk->queue);
 	sectors = 0;
 
-	if (index >= bio->bi_idx)
-		index = bio->bi_vcnt - 1;
+	if (index > bio->bi_vcnt)
+		return 0;
 
 	__bio_for_each_segment(bv, bio, i, 0) {
 		if (i == index) {
#1 lore raw
Quoted 2 lines · Martin K. Petersen

DM would occasionally end up splitting data integrity-enabled requests incorrectly. The culprit was a bad range check in bio_sector_offset.

The patch looks ok to me, but what is the user visible behavior when this happens?

Cheers,
Jeff

author
#2 lore raw
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Jeff" == Jeff Moyer <[email protected]> writes:

DM would occasionally end up splitting data integrity-enabled requests incorrectly. The culprit was a bad range check in bio_sector_offset.

Jeff> The patch looks ok to me, but what is the user visible behavior Jeff> when this happens?

We'd occasionally end up mapping a bad integrity scatterlist and the HBA would abort the I/O with a protection information error.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
Acked-by
#3 lore raw
Quoted 11 lines · Martin K. Petersen
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Jeff" == Jeff Moyer <[email protected]> writes:

DM would occasionally end up splitting data integrity-enabled requests incorrectly. The culprit was a bad range check in bio_sector_offset.

Jeff> The patch looks ok to me, but what is the user visible behavior Jeff> when this happens?

We'd occasionally end up mapping a bad integrity scatterlist and the HBA would abort the I/O with a protection information error.

Thanks for the explanation, Martin!

Acked-by Jeff Moyer <[email protected]>
#4 lore raw
In reply to Martin K. Petersen
Quoted 12 lines · Martin K. Petersen

DM would occasionally end up splitting data integrity-enabled requests incorrectly. The culprit was a bad range check in bio_sector_offset.

Signed-off-by Martin K. Petersen <[email protected]>
Cc <[email protected]>
fs/bio.c +0 -0
index 9bfade8..b9a6744 100644

fs/bio.c sector_t bio_sector_offset(struct bio *bio, unsigned short index, · line 1559

Quoted 10 lines
fs/bio.c +2 -2
@@ -1552,8 +1552,8 @@ sector_t bio_sector_offset(struct bio *bio, unsigned short index,
 	sector_sz = queue_logical_block_size(bio->bi_bdev->bd_disk->queue);
 	sectors = 0;
 
-	if (index >= bio->bi_idx)
-		index = bio->bi_vcnt - 1;
+	if (index > bio->bi_vcnt)
+		return 0;
 
 	__bio_for_each_segment(bv, bio, i, 0) {
 		if (i == index) {

Good catch, merged.

Signature · 1 line
Jens Axboe