From: Minwoo Im <minwoo.im.dev@gmail.com>
To: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Christoph Hellwig <hch@lst.de>,
Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
Minwoo Im <minwoo.im.dev@gmail.com>
Subject: [RFC PATCH V3 1/1] block: reject I/O for same fd if block size changed
Date: Mon, 4 Jan 2021 22:06:59 +0900 [thread overview]
Message-ID: <20210104130659.22511-2-minwoo.im.dev@gmail.com> (raw)
In-Reply-To: <20210104130659.22511-1-minwoo.im.dev@gmail.com>
This patch fixes I/O errors during BLKRRPART ioctl() behavior right
after format operation that changed logical block size of the block
device with a same file descriptor opened.
This issue can be easily reproduced with a single format command in case
of NVMe (logical block size 512B to 4096B).
nvme format /dev/nvme0n1 --lbaf=1 --force
This is because the application, nvme-cli format subcommand issues an
admin command followed by BLKRRPART ioctl to re-read partition
information without closing the file descriptor. If file descriptor
stays opened, __blkdev_get() will not be invoked at all even logical
block size has been changed.
It will cause I/O errors with invalid Read operations during the
BLKRRPART ioctl due to i_blkbits mismatch. The invalid operations in
BLKRRPART happens with under-flowed Number of LBA(NLB) values
0xffff(65535) because i_blkbits is still set to 9 even the logical block
size has been updated to 4096. The BLKRRPART will lead buffer_head to
hold 512B data which is less than the logical lock size of the block
device.
The root cause, which is because i_blkbits of inode of the block device
is not updated, can be solved easily by re-opening file descriptor
again from application. But, that's just for application's business
and kernel should reject invalid Read operations during the BLKRRPART
ioctl.
This patch rejects I/O from the path of add_partitions() to avoid
issuing invalid Read operations to device. It also sets a flag to
gendisk in blk_queue_logical_block_size to minimize caller-side updates.
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
block/blk-settings.c | 8 ++++++++
block/partitions/core.c | 11 +++++++++++
fs/block_dev.c | 6 ++++++
include/linux/genhd.h | 1 +
4 files changed, 26 insertions(+)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 43990b1d148b..84136ea4e2a4 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -328,6 +328,14 @@ EXPORT_SYMBOL(blk_queue_max_segment_size);
void blk_queue_logical_block_size(struct request_queue *q, unsigned int size)
{
struct queue_limits *limits = &q->limits;
+ struct block_device *bdev;
+
+ if (q->backing_dev_info && q->backing_dev_info->owner &&
+ limits->logical_block_size != size) {
+ bdev = blkdev_get_no_open(q->backing_dev_info->owner->devt);
+ bdev->bd_disk->flags |= GENHD_FL_BLOCK_SIZE_CHANGED;
+ blkdev_put_no_open(bdev);
+ }
limits->logical_block_size = size;
diff --git a/block/partitions/core.c b/block/partitions/core.c
index e7d776db803b..5a0330c1b6f9 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -618,6 +618,17 @@ int blk_add_partitions(struct gendisk *disk, struct block_device *bdev)
if (!disk_part_scan_enabled(disk))
return 0;
+ /*
+ * Reject to check partition information if block size has been changed
+ * in the runtime. If block size of a block device has been changed,
+ * the file descriptor should be opened agian to update the blkbits.
+ */
+ if (disk->flags & GENHD_FL_BLOCK_SIZE_CHANGED) {
+ pr_warn("%s: rejecting checking partition. fd should be opened again.\n",
+ disk->disk_name);
+ return -EBADFD;
+ }
+
state = check_partition(disk, bdev);
if (!state)
return 0;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9293045e128c..c996de3d6084 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -131,6 +131,12 @@ EXPORT_SYMBOL(truncate_bdev_range);
static void set_init_blocksize(struct block_device *bdev)
{
bdev->bd_inode->i_blkbits = blksize_bits(bdev_logical_block_size(bdev));
+
+ /*
+ * Allow I/O commands for this block device. We can say that this
+ * block device has been set to a proper block size.
+ */
+ bdev->bd_disk->flags &= ~GENHD_FL_BLOCK_SIZE_CHANGED;
}
int set_blocksize(struct block_device *bdev, int size)
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 809aaa32d53c..0e0e24917003 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -103,6 +103,7 @@ struct partition_meta_info {
#define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE 0x0100
#define GENHD_FL_NO_PART_SCAN 0x0200
#define GENHD_FL_HIDDEN 0x0400
+#define GENHD_FL_BLOCK_SIZE_CHANGED 0x0800
enum {
DISK_EVENT_MEDIA_CHANGE = 1 << 0, /* media changed */
--
2.17.1
next prev parent reply other threads:[~2021-01-04 13:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-04 13:06 [RFC PATCH V3 0/1] block: fix I/O errors in BLKRRPART Minwoo Im
2021-01-04 13:06 ` Minwoo Im [this message]
2021-01-04 17:11 ` [RFC PATCH V3 1/1] block: reject I/O for same fd if block size changed Christoph Hellwig
2021-01-04 17:11 ` Christoph Hellwig
2021-01-05 1:04 ` Minwoo Im
2021-01-05 7:50 ` Christoph Hellwig
2021-01-05 10:12 ` Minwoo Im
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210104130659.22511-2-minwoo.im.dev@gmail.com \
--to=minwoo.im.dev@gmail.com \
--cc=Chaitanya.Kulkarni@wdc.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).