From: Minwoo Im <minwoo.im.dev@gmail.com> To: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org, linux-fsdevel@vger.kernel.org Cc: Jens Axboe <axboe@kernel.dk>, Alexander Viro <viro@zeniv.linux.org.uk>, Christoph Hellwig <hch@lst.de>, Minwoo Im <minwoo.im.dev@gmail.com> Subject: [PATCH V6 1/1] block: reject I/O for same fd if block size changed Date: Tue, 26 Jan 2021 09:29:01 +0900 [thread overview] Message-ID: <20210126002901.5533-2-minwoo.im.dev@gmail.com> (raw) In-Reply-To: <20210126002901.5533-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 sets a flag to request_queue in blk_queue_logical_block_size to minimize caller-side updates. Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com> Reviewed-by: Christoph Hellwig <hch@lst.de> --- block/blk-settings.c | 3 +++ block/partitions/core.c | 12 ++++++++++++ fs/block_dev.c | 8 ++++++++ include/linux/blkdev.h | 1 + 4 files changed, 24 insertions(+) diff --git a/block/blk-settings.c b/block/blk-settings.c index 4c974340f1a9..5b1e0bf54d7e 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -329,6 +329,9 @@ void blk_queue_logical_block_size(struct request_queue *q, unsigned int size) { struct queue_limits *limits = &q->limits; + if (limits->logical_block_size != size) + blk_queue_flag_set(QUEUE_FLAG_LBSZ_CHANGED, q); + limits->logical_block_size = size; if (limits->physical_block_size < size) diff --git a/block/partitions/core.c b/block/partitions/core.c index b1cdf88f96e2..092d6712c7fc 100644 --- a/block/partitions/core.c +++ b/block/partitions/core.c @@ -601,12 +601,24 @@ static bool blk_add_partition(struct gendisk *disk, struct block_device *bdev, int blk_add_partitions(struct gendisk *disk, struct block_device *bdev) { + struct request_queue *q = bdev_get_queue(bdev); struct parsed_partitions *state; int ret = -EAGAIN, p; 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 (test_bit(QUEUE_FLAG_LBSZ_CHANGED, &q->queue_flags)) { + 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 6f5bd9950baf..8555308c503d 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -130,7 +130,15 @@ EXPORT_SYMBOL(truncate_bdev_range); static void set_init_blocksize(struct block_device *bdev) { + struct request_queue *q = bdev_get_queue(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 proper blkbits updated. + */ + blk_queue_flag_clear(QUEUE_FLAG_LBSZ_CHANGED, q); } int set_blocksize(struct block_device *bdev, int size) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 20f3706b6b2e..f725e933db40 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -621,6 +621,7 @@ struct request_queue { #define QUEUE_FLAG_RQ_ALLOC_TIME 27 /* record rq->alloc_time_ns */ #define QUEUE_FLAG_HCTX_ACTIVE 28 /* at least one blk-mq hctx is active */ #define QUEUE_FLAG_NOWAIT 29 /* device supports NOWAIT */ +#define QUEUE_FLAG_LBSZ_CHANGED 30 /* logical block size changed */ #define QUEUE_FLAG_MQ_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ (1 << QUEUE_FLAG_SAME_COMP) | \ -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Minwoo Im <minwoo.im.dev@gmail.com> To: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org, linux-fsdevel@vger.kernel.org Cc: Jens Axboe <axboe@kernel.dk>, Minwoo Im <minwoo.im.dev@gmail.com>, Alexander Viro <viro@zeniv.linux.org.uk>, Christoph Hellwig <hch@lst.de> Subject: [PATCH V6 1/1] block: reject I/O for same fd if block size changed Date: Tue, 26 Jan 2021 09:29:01 +0900 [thread overview] Message-ID: <20210126002901.5533-2-minwoo.im.dev@gmail.com> (raw) In-Reply-To: <20210126002901.5533-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 sets a flag to request_queue in blk_queue_logical_block_size to minimize caller-side updates. Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com> Reviewed-by: Christoph Hellwig <hch@lst.de> --- block/blk-settings.c | 3 +++ block/partitions/core.c | 12 ++++++++++++ fs/block_dev.c | 8 ++++++++ include/linux/blkdev.h | 1 + 4 files changed, 24 insertions(+) diff --git a/block/blk-settings.c b/block/blk-settings.c index 4c974340f1a9..5b1e0bf54d7e 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -329,6 +329,9 @@ void blk_queue_logical_block_size(struct request_queue *q, unsigned int size) { struct queue_limits *limits = &q->limits; + if (limits->logical_block_size != size) + blk_queue_flag_set(QUEUE_FLAG_LBSZ_CHANGED, q); + limits->logical_block_size = size; if (limits->physical_block_size < size) diff --git a/block/partitions/core.c b/block/partitions/core.c index b1cdf88f96e2..092d6712c7fc 100644 --- a/block/partitions/core.c +++ b/block/partitions/core.c @@ -601,12 +601,24 @@ static bool blk_add_partition(struct gendisk *disk, struct block_device *bdev, int blk_add_partitions(struct gendisk *disk, struct block_device *bdev) { + struct request_queue *q = bdev_get_queue(bdev); struct parsed_partitions *state; int ret = -EAGAIN, p; 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 (test_bit(QUEUE_FLAG_LBSZ_CHANGED, &q->queue_flags)) { + 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 6f5bd9950baf..8555308c503d 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -130,7 +130,15 @@ EXPORT_SYMBOL(truncate_bdev_range); static void set_init_blocksize(struct block_device *bdev) { + struct request_queue *q = bdev_get_queue(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 proper blkbits updated. + */ + blk_queue_flag_clear(QUEUE_FLAG_LBSZ_CHANGED, q); } int set_blocksize(struct block_device *bdev, int size) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 20f3706b6b2e..f725e933db40 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -621,6 +621,7 @@ struct request_queue { #define QUEUE_FLAG_RQ_ALLOC_TIME 27 /* record rq->alloc_time_ns */ #define QUEUE_FLAG_HCTX_ACTIVE 28 /* at least one blk-mq hctx is active */ #define QUEUE_FLAG_NOWAIT 29 /* device supports NOWAIT */ +#define QUEUE_FLAG_LBSZ_CHANGED 30 /* logical block size changed */ #define QUEUE_FLAG_MQ_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ (1 << QUEUE_FLAG_SAME_COMP) | \ -- 2.17.1 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2021-01-26 5:35 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-26 0:29 [PATCH V6 0/1] block: fix I/O errors in BLKRRPART Minwoo Im 2021-01-26 0:29 ` Minwoo Im 2021-01-26 0:29 ` Minwoo Im [this message] 2021-01-26 0:29 ` [PATCH V6 1/1] block: reject I/O for same fd if block size changed 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=20210126002901.5533-2-minwoo.im.dev@gmail.com \ --to=minwoo.im.dev@gmail.com \ --cc=axboe@kernel.dk \ --cc=hch@lst.de \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-nvme@lists.infradead.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.