Skip to contentSkip to conversation

brd: Refuse improperly aligned discard requests

PATCH Applied Jan Kara 4 participants 4 messages Nov 4, 2015 – Nov 5, 2015

Subsystems (3)
VFS Block layer Ramdisk RAM block device driver
lore
Share & tools
lore

Jan Kara

author
#0 lore raw

PATCH 1/1 brd: Refuse improperly aligned discard requests

Currently when improperly aligned discard request is submitted, we just silently discard more / less data which results in filesystem corruption in some cases. Refuse such misaligned requests.

Signed-off-by Jan Kara <[email protected]>
---
 drivers/block/brd.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/block/brd.c +3 -0
index b9794aeeb878..4ef4cdf67ede 100644
@@ -337,6 +337,9 @@ static void brd_make_request(struct request_queue *q, struct bio *bio)
 		goto io_error;
 
 	if (unlikely(bio->bi_rw & REQ_DISCARD)) {
+		if (sector & ((PAGE_SIZE >> SECTOR_SHIFT) - 1) ||
+		    bio->bi_iter.bi_size & PAGE_MASK)
+			goto io_error;
 		discard_from_brd(brd, sector, bio->bi_iter.bi_size);
 		goto out;
 	}
Signature · 1 line
2.1.4
#1 lore raw
Quoted 10 lines

-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Jan Kara
Sent: Wednesday, November 4, 2015 10:14 AM
To: [email protected]
Cc: LKML <[email protected]>; [email protected]; Christoph Hellwig <[email protected]>; Jan Kara <[email protected]>
Subject: [PATCH] brd: Refuse improperly aligned discard requests

Quoted 3 lines

Currently when improperly aligned discard request is submitted, we just silently discard more / less data which results in filesystem corruption in some cases. Refuse such misaligned requests.

I agree discarding more than requested is very bad.

If they are routed to SCSI or ATA devices, though, the discard commands (SCSI UNMAP or ATA DATA SET MANAGEMENT/TRIM) are just hints, so there is no guarantee the discard will do anything. Are you finding
filesystems that still don't understand that? dm-raid held that
mistaken assumption for a long time.

---
Robert Elliott, HP Server Storage

reviewer
#2 lore raw
Quoted 14 lines · Robert (Persistent Memory)
Quoted 10 lines

-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Jan Kara
Sent: Wednesday, November 4, 2015 10:14 AM
To: [email protected]
Cc: LKML <[email protected]>; [email protected]; Christoph Hellwig <[email protected]>; Jan Kara <[email protected]>
Subject: [PATCH] brd: Refuse improperly aligned discard requests

Quoted 3 lines

Currently when improperly aligned discard request is submitted, we just silently discard more / less data which results in filesystem corruption in some cases. Refuse such misaligned requests.

Quoted 7 lines

I agree discarding more than requested is very bad.

If they are routed to SCSI or ATA devices, though, the discard commands (SCSI UNMAP or ATA DATA SET MANAGEMENT/TRIM) are just hints, so there is no guarantee the discard will do anything. Are you finding
filesystems that still don't understand that? dm-raid held that
mistaken assumption for a long time.

So there is blkdev_issue_zeroout() which uses blkdev_issue_discard() if 'discard_zeroes_data' is set. I'd hope that in that case zeroing really happens as submitted or error is returned... Filesystems definitely depend on blkdev_issue_zeroout() doing what it is told.

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
maintainer Applied
#3 lore raw
In reply to Jan Kara
Quoted 3 lines · Jan Kara

Currently when improperly aligned discard request is submitted, we just silently discard more / less data which results in filesystem corruption in some cases. Refuse such misaligned requests.

Applied, thanks Jan.

Signature · 1 line
Jens Axboe