* [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size
@ 2021-10-26 14:40 Xie Yongji
2021-10-26 14:40 ` [PATCH v3 1/4] block: Add a helper to validate the " Xie Yongji
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Xie Yongji @ 2021-10-26 14:40 UTC (permalink / raw)
To: axboe, hch, josef, mst, jasowang, stefanha, kwolf
Cc: linux-block, nbd, virtualization
The block layer can't support the block size larger than
page size yet, so driver needs to validate the block size
before setting it. Now this validation is done in device drivers
with some duplicated codes. This series tries to add a block
layer helper for it and makes loop driver, nbd driver and
virtio-blk driver use it.
We tested both invalid block size and valid block size cases
for loop driver with the example in man page [1] and for
nbd and virtio-blk driver with qemu-nbd and storage-daemon (vduse)
in the qemu repo [2].
[1] https://man7.org/linux/man-pages/man4/loop.4.html
[2] https://github.com/bytedance/qemu/tree/vduse
V2 to V3:
- Fix some commit logs and print message
V1 to V2:
- Return and print error if validation fails in virtio-blk driver
Xie Yongji (4):
block: Add a helper to validate the block size
nbd: Use blk_validate_block_size() to validate block size
loop: Use blk_validate_block_size() to validate block size
virtio-blk: Use blk_validate_block_size() to validate block size
drivers/block/loop.c | 17 ++---------------
drivers/block/nbd.c | 3 ++-
drivers/block/virtio_blk.c | 12 ++++++++++--
include/linux/blkdev.h | 8 ++++++++
4 files changed, 22 insertions(+), 18 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/4] block: Add a helper to validate the block size
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
@ 2021-10-26 14:40 ` Xie Yongji
2021-10-26 14:40 ` [PATCH v3 2/4] nbd: Use blk_validate_block_size() to validate " Xie Yongji
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Xie Yongji @ 2021-10-26 14:40 UTC (permalink / raw)
To: axboe, hch, josef, mst, jasowang, stefanha, kwolf
Cc: linux-block, nbd, virtualization
There are some duplicated codes to validate the block
size in block drivers. This limitation actually comes
from block layer, so this patch tries to add a new block
layer helper for that.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
include/linux/blkdev.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 12b9dbcc980e..805cd02d7914 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -235,6 +235,14 @@ struct request {
void *end_io_data;
};
+static inline int blk_validate_block_size(unsigned int bsize)
+{
+ if (bsize < 512 || bsize > PAGE_SIZE || !is_power_of_2(bsize))
+ return -EINVAL;
+
+ return 0;
+}
+
static inline bool blk_op_is_passthrough(unsigned int op)
{
op &= REQ_OP_MASK;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/4] nbd: Use blk_validate_block_size() to validate block size
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
2021-10-26 14:40 ` [PATCH v3 1/4] block: Add a helper to validate the " Xie Yongji
@ 2021-10-26 14:40 ` Xie Yongji
2021-10-26 14:40 ` [PATCH v3 3/4] loop: " Xie Yongji
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Xie Yongji @ 2021-10-26 14:40 UTC (permalink / raw)
To: axboe, hch, josef, mst, jasowang, stefanha, kwolf
Cc: linux-block, nbd, virtualization
Use the block layer helper to validate block size instead
of open coding it.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/block/nbd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 1183f7872b71..3f58c3eb38b6 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -323,7 +323,8 @@ static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize,
{
if (!blksize)
blksize = 1u << NBD_DEF_BLKSIZE_BITS;
- if (blksize < 512 || blksize > PAGE_SIZE || !is_power_of_2(blksize))
+
+ if (blk_validate_block_size(blksize))
return -EINVAL;
nbd->config->bytesize = bytesize;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/4] loop: Use blk_validate_block_size() to validate block size
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
2021-10-26 14:40 ` [PATCH v3 1/4] block: Add a helper to validate the " Xie Yongji
2021-10-26 14:40 ` [PATCH v3 2/4] nbd: Use blk_validate_block_size() to validate " Xie Yongji
@ 2021-10-26 14:40 ` Xie Yongji
2021-10-26 14:40 ` [PATCH v3 4/4] virtio-blk: " Xie Yongji
2021-10-27 20:16 ` Jens Axboe
4 siblings, 0 replies; 9+ messages in thread
From: Xie Yongji @ 2021-10-26 14:40 UTC (permalink / raw)
To: axboe, hch, josef, mst, jasowang, stefanha, kwolf
Cc: linux-block, nbd, virtualization
Remove loop_validate_block_size() and use the block layer helper
to validate block size.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/block/loop.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 7bf4686af774..dfc72a1f6500 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -273,19 +273,6 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
}
/**
- * loop_validate_block_size() - validates the passed in block size
- * @bsize: size to validate
- */
-static int
-loop_validate_block_size(unsigned short bsize)
-{
- if (bsize < 512 || bsize > PAGE_SIZE || !is_power_of_2(bsize))
- return -EINVAL;
-
- return 0;
-}
-
-/**
* loop_set_size() - sets device size and notifies userspace
* @lo: struct loop_device to set the size for
* @size: new size of the loop device
@@ -1236,7 +1223,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
}
if (config->block_size) {
- error = loop_validate_block_size(config->block_size);
+ error = blk_validate_block_size(config->block_size);
if (error)
goto out_unlock;
}
@@ -1759,7 +1746,7 @@ static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
if (lo->lo_state != Lo_bound)
return -ENXIO;
- err = loop_validate_block_size(arg);
+ err = blk_validate_block_size(arg);
if (err)
return err;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 4/4] virtio-blk: Use blk_validate_block_size() to validate block size
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
` (2 preceding siblings ...)
2021-10-26 14:40 ` [PATCH v3 3/4] loop: " Xie Yongji
@ 2021-10-26 14:40 ` Xie Yongji
2021-10-27 19:58 ` Michael S. Tsirkin
2021-10-27 20:16 ` Jens Axboe
4 siblings, 1 reply; 9+ messages in thread
From: Xie Yongji @ 2021-10-26 14:40 UTC (permalink / raw)
To: axboe, hch, josef, mst, jasowang, stefanha, kwolf
Cc: linux-block, nbd, virtualization
The block layer can't support a block size larger than
page size yet. And a block size that's too small or
not a power of two won't work either. If a misconfigured
device presents an invalid block size in configuration space,
it will result in the kernel crash something like below:
[ 506.154324] BUG: kernel NULL pointer dereference, address: 0000000000000008
[ 506.160416] RIP: 0010:create_empty_buffers+0x24/0x100
[ 506.174302] Call Trace:
[ 506.174651] create_page_buffers+0x4d/0x60
[ 506.175207] block_read_full_page+0x50/0x380
[ 506.175798] ? __mod_lruvec_page_state+0x60/0xa0
[ 506.176412] ? __add_to_page_cache_locked+0x1b2/0x390
[ 506.177085] ? blkdev_direct_IO+0x4a0/0x4a0
[ 506.177644] ? scan_shadow_nodes+0x30/0x30
[ 506.178206] ? lru_cache_add+0x42/0x60
[ 506.178716] do_read_cache_page+0x695/0x740
[ 506.179278] ? read_part_sector+0xe0/0xe0
[ 506.179821] read_part_sector+0x36/0xe0
[ 506.180337] adfspart_check_ICS+0x32/0x320
[ 506.180890] ? snprintf+0x45/0x70
[ 506.181350] ? read_part_sector+0xe0/0xe0
[ 506.181906] bdev_disk_changed+0x229/0x5c0
[ 506.182483] blkdev_get_whole+0x6d/0x90
[ 506.183013] blkdev_get_by_dev+0x122/0x2d0
[ 506.183562] device_add_disk+0x39e/0x3c0
[ 506.184472] virtblk_probe+0x3f8/0x79b [virtio_blk]
[ 506.185461] virtio_dev_probe+0x15e/0x1d0 [virtio]
So let's use a block layer helper to validate the block size.
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
drivers/block/virtio_blk.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 303caf2d17d0..fd086179f980 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -815,9 +815,17 @@ static int virtblk_probe(struct virtio_device *vdev)
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
struct virtio_blk_config, blk_size,
&blk_size);
- if (!err)
+ if (!err) {
+ err = blk_validate_block_size(blk_size);
+ if (err) {
+ dev_err(&vdev->dev,
+ "virtio_blk: invalid block size: 0x%x\n",
+ blk_size);
+ goto out_cleanup_disk;
+ }
+
blk_queue_logical_block_size(q, blk_size);
- else
+ } else
blk_size = queue_logical_block_size(q);
/* Use topology information if available */
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 4/4] virtio-blk: Use blk_validate_block_size() to validate block size
2021-10-26 14:40 ` [PATCH v3 4/4] virtio-blk: " Xie Yongji
@ 2021-10-27 19:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2021-10-27 19:58 UTC (permalink / raw)
To: Xie Yongji
Cc: axboe, hch, josef, jasowang, stefanha, kwolf, linux-block, nbd,
virtualization
On Tue, Oct 26, 2021 at 10:40:15PM +0800, Xie Yongji wrote:
> The block layer can't support a block size larger than
> page size yet. And a block size that's too small or
> not a power of two won't work either. If a misconfigured
> device presents an invalid block size in configuration space,
> it will result in the kernel crash something like below:
>
> [ 506.154324] BUG: kernel NULL pointer dereference, address: 0000000000000008
> [ 506.160416] RIP: 0010:create_empty_buffers+0x24/0x100
> [ 506.174302] Call Trace:
> [ 506.174651] create_page_buffers+0x4d/0x60
> [ 506.175207] block_read_full_page+0x50/0x380
> [ 506.175798] ? __mod_lruvec_page_state+0x60/0xa0
> [ 506.176412] ? __add_to_page_cache_locked+0x1b2/0x390
> [ 506.177085] ? blkdev_direct_IO+0x4a0/0x4a0
> [ 506.177644] ? scan_shadow_nodes+0x30/0x30
> [ 506.178206] ? lru_cache_add+0x42/0x60
> [ 506.178716] do_read_cache_page+0x695/0x740
> [ 506.179278] ? read_part_sector+0xe0/0xe0
> [ 506.179821] read_part_sector+0x36/0xe0
> [ 506.180337] adfspart_check_ICS+0x32/0x320
> [ 506.180890] ? snprintf+0x45/0x70
> [ 506.181350] ? read_part_sector+0xe0/0xe0
> [ 506.181906] bdev_disk_changed+0x229/0x5c0
> [ 506.182483] blkdev_get_whole+0x6d/0x90
> [ 506.183013] blkdev_get_by_dev+0x122/0x2d0
> [ 506.183562] device_add_disk+0x39e/0x3c0
> [ 506.184472] virtblk_probe+0x3f8/0x79b [virtio_blk]
> [ 506.185461] virtio_dev_probe+0x15e/0x1d0 [virtio]
>
> So let's use a block layer helper to validate the block size.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Please merge through the block tree because of the
dependency.
Jens can you pick this up?
> ---
> drivers/block/virtio_blk.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 303caf2d17d0..fd086179f980 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -815,9 +815,17 @@ static int virtblk_probe(struct virtio_device *vdev)
> err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
> struct virtio_blk_config, blk_size,
> &blk_size);
> - if (!err)
> + if (!err) {
> + err = blk_validate_block_size(blk_size);
> + if (err) {
> + dev_err(&vdev->dev,
> + "virtio_blk: invalid block size: 0x%x\n",
> + blk_size);
> + goto out_cleanup_disk;
> + }
> +
> blk_queue_logical_block_size(q, blk_size);
> - else
> + } else
> blk_size = queue_logical_block_size(q);
>
> /* Use topology information if available */
> --
> 2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 4/4] virtio-blk: Use blk_validate_block_size() to validate block size
@ 2021-10-27 19:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2021-10-27 19:58 UTC (permalink / raw)
To: Xie Yongji; +Cc: axboe, josef, nbd, linux-block, stefanha, virtualization, hch
On Tue, Oct 26, 2021 at 10:40:15PM +0800, Xie Yongji wrote:
> The block layer can't support a block size larger than
> page size yet. And a block size that's too small or
> not a power of two won't work either. If a misconfigured
> device presents an invalid block size in configuration space,
> it will result in the kernel crash something like below:
>
> [ 506.154324] BUG: kernel NULL pointer dereference, address: 0000000000000008
> [ 506.160416] RIP: 0010:create_empty_buffers+0x24/0x100
> [ 506.174302] Call Trace:
> [ 506.174651] create_page_buffers+0x4d/0x60
> [ 506.175207] block_read_full_page+0x50/0x380
> [ 506.175798] ? __mod_lruvec_page_state+0x60/0xa0
> [ 506.176412] ? __add_to_page_cache_locked+0x1b2/0x390
> [ 506.177085] ? blkdev_direct_IO+0x4a0/0x4a0
> [ 506.177644] ? scan_shadow_nodes+0x30/0x30
> [ 506.178206] ? lru_cache_add+0x42/0x60
> [ 506.178716] do_read_cache_page+0x695/0x740
> [ 506.179278] ? read_part_sector+0xe0/0xe0
> [ 506.179821] read_part_sector+0x36/0xe0
> [ 506.180337] adfspart_check_ICS+0x32/0x320
> [ 506.180890] ? snprintf+0x45/0x70
> [ 506.181350] ? read_part_sector+0xe0/0xe0
> [ 506.181906] bdev_disk_changed+0x229/0x5c0
> [ 506.182483] blkdev_get_whole+0x6d/0x90
> [ 506.183013] blkdev_get_by_dev+0x122/0x2d0
> [ 506.183562] device_add_disk+0x39e/0x3c0
> [ 506.184472] virtblk_probe+0x3f8/0x79b [virtio_blk]
> [ 506.185461] virtio_dev_probe+0x15e/0x1d0 [virtio]
>
> So let's use a block layer helper to validate the block size.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Please merge through the block tree because of the
dependency.
Jens can you pick this up?
> ---
> drivers/block/virtio_blk.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 303caf2d17d0..fd086179f980 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -815,9 +815,17 @@ static int virtblk_probe(struct virtio_device *vdev)
> err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
> struct virtio_blk_config, blk_size,
> &blk_size);
> - if (!err)
> + if (!err) {
> + err = blk_validate_block_size(blk_size);
> + if (err) {
> + dev_err(&vdev->dev,
> + "virtio_blk: invalid block size: 0x%x\n",
> + blk_size);
> + goto out_cleanup_disk;
> + }
> +
> blk_queue_logical_block_size(q, blk_size);
> - else
> + } else
> blk_size = queue_logical_block_size(q);
>
> /* Use topology information if available */
> --
> 2.11.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
@ 2021-10-27 20:16 ` Jens Axboe
2021-10-26 14:40 ` [PATCH v3 2/4] nbd: Use blk_validate_block_size() to validate " Xie Yongji
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-10-27 20:16 UTC (permalink / raw)
To: kwolf, hch, stefanha, jasowang, mst, Xie Yongji, josef
Cc: virtualization, nbd, linux-block
On Tue, 26 Oct 2021 22:40:11 +0800, Xie Yongji wrote:
> The block layer can't support the block size larger than
> page size yet, so driver needs to validate the block size
> before setting it. Now this validation is done in device drivers
> with some duplicated codes. This series tries to add a block
> layer helper for it and makes loop driver, nbd driver and
> virtio-blk driver use it.
>
> [...]
Applied, thanks!
[1/4] block: Add a helper to validate the block size
commit: 570b1cac477643cbf01a45fa5d018430a1fddbce
[2/4] nbd: Use blk_validate_block_size() to validate block size
commit: c4318d6cd038472d13e08a54a9035863c8c160bd
[3/4] loop: Use blk_validate_block_size() to validate block size
commit: af3c570fb0df422b4906ebd11c1bf363d89961d5
[4/4] virtio-blk: Use blk_validate_block_size() to validate block size
commit: 57a13a5b8157d9a8606490aaa1b805bafe6c37e1
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size
@ 2021-10-27 20:16 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2021-10-27 20:16 UTC (permalink / raw)
To: kwolf, hch, stefanha, jasowang, mst, Xie Yongji, josef
Cc: linux-block, nbd, virtualization
On Tue, 26 Oct 2021 22:40:11 +0800, Xie Yongji wrote:
> The block layer can't support the block size larger than
> page size yet, so driver needs to validate the block size
> before setting it. Now this validation is done in device drivers
> with some duplicated codes. This series tries to add a block
> layer helper for it and makes loop driver, nbd driver and
> virtio-blk driver use it.
>
> [...]
Applied, thanks!
[1/4] block: Add a helper to validate the block size
commit: 570b1cac477643cbf01a45fa5d018430a1fddbce
[2/4] nbd: Use blk_validate_block_size() to validate block size
commit: c4318d6cd038472d13e08a54a9035863c8c160bd
[3/4] loop: Use blk_validate_block_size() to validate block size
commit: af3c570fb0df422b4906ebd11c1bf363d89961d5
[4/4] virtio-blk: Use blk_validate_block_size() to validate block size
commit: 57a13a5b8157d9a8606490aaa1b805bafe6c37e1
Best regards,
--
Jens Axboe
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-27 20:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 14:40 [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers to validate block size Xie Yongji
2021-10-26 14:40 ` [PATCH v3 1/4] block: Add a helper to validate the " Xie Yongji
2021-10-26 14:40 ` [PATCH v3 2/4] nbd: Use blk_validate_block_size() to validate " Xie Yongji
2021-10-26 14:40 ` [PATCH v3 3/4] loop: " Xie Yongji
2021-10-26 14:40 ` [PATCH v3 4/4] virtio-blk: " Xie Yongji
2021-10-27 19:58 ` Michael S. Tsirkin
2021-10-27 19:58 ` Michael S. Tsirkin
2021-10-27 20:16 ` [PATCH v3 0/4] Add blk_validate_block_size() helper for drivers " Jens Axboe
2021-10-27 20:16 ` Jens Axboe
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.