Hi, This has triggered in 5.10.77 yesterday [1], and I was able to reproduce it on 5.10.80 using the C repro from android-54 [2]. What happens is that the function do_mpage_readpage() calls bdev_read_page() [3] passing in bdev == NULL, and bdev_read_page() crashes here [4]. This happens in 5.15 down to 5.10, but it is fixed in 5.16-rc1. I bisected it to the first good commit, which is:
af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size")
The root cause seems to be loss of precision in loop_configure(), when it calls loop_validate_block_size() in [5]. The config->block_size is an uint32 and the bsize param passed to loop_validate_block_size() is unsigned short. The reproducer sets up a loop device with the block size equal to 0x20000400, which is bigger than USHRT_MAX. The loop_validate_block_size() returns 0, but uses the invalid size to setup the device. The new helper changes the bsize param type to uint, and the issue goes away.
To fix this for the older kernels can we please have the two commits:
570b1cac4776 ("block: Add a helper to validate the block size") af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size")
applied to 5.15, 5.14, and 5.10. The first one needs to be back ported, but the second applies cleanly. I will follow up back ports for each version in few minutes.
From: Xie Yongji xieyongji@bytedance.com
From: Xie Yongji xieyongji@bytedance.com
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 Link: https://lore.kernel.org/r/20211026144015.188-2-xieyongji@bytedance.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Tadeusz Struk tadeusz.struk@linaro.org --- include/linux/blkdev.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 683aee365420..5b03795ae33b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -54,6 +54,14 @@ struct blk_keyslot_manager; */ #define BLKCG_MAX_POLS 6
+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; +} + typedef void (rq_end_io_fn)(struct request *, blk_status_t);
/*
On 2021/11/19 8:57, Tadeusz Struk wrote:
From: Xie Yongji xieyongji@bytedance.com
From: Xie Yongji xieyongji@bytedance.com
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 Link: https://lore.kernel.org/r/20211026144015.188-2-xieyongji@bytedance.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Tadeusz Struk tadeusz.struk@linaro.org
include/linux/blkdev.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 683aee365420..5b03795ae33b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -54,6 +54,14 @@ struct blk_keyslot_manager; */ #define BLKCG_MAX_POLS 6 +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;
+}
typedef void (rq_end_io_fn)(struct request *, blk_status_t); /*
But where is this used in 5.15 ? I do not see any callers for this. So why backport it ?
On 11/18/21 17:55, Damien Le Moal wrote:
But where is this used in 5.15 ? I do not see any callers for this. So why backport it ?
It will be used after the af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size") is applied. Please read the first message in the thread to get the context.
On 2021/11/19 11:41, Tadeusz Struk wrote:
On 11/18/21 17:55, Damien Le Moal wrote:
But where is this used in 5.15 ? I do not see any callers for this. So why backport it ?
It will be used after the af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size") is applied. Please read the first message in the thread to get the context.
None of the patches in that series are marked for stable. If you want all of them backported, sending all patches together as a series would make things easier to understand.
From: Xie Yongji xieyongji@bytedance.com
From: Xie Yongji xieyongji@bytedance.com
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 Link: https://lore.kernel.org/r/20211026144015.188-2-xieyongji@bytedance.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Tadeusz Struk tadeusz.struk@linaro.org --- include/linux/blkdev.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index e7979fe7e4fa..84019910446a 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -59,6 +59,14 @@ struct blk_keyslot_manager; */ #define BLKCG_MAX_POLS 6
+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; +} + typedef void (rq_end_io_fn)(struct request *, blk_status_t);
/*
From: Xie Yongji xieyongji@bytedance.com
From: Xie Yongji xieyongji@bytedance.com
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 Link: https://lore.kernel.org/r/20211026144015.188-2-xieyongji@bytedance.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Tadeusz Struk tadeusz.struk@linaro.org --- include/linux/blkdev.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 4ba17736b614..98fdf5a31fd6 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -59,6 +59,14 @@ struct blk_keyslot_manager; */ #define BLKCG_MAX_POLS 5
+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; +} + typedef void (rq_end_io_fn)(struct request *, blk_status_t);
/*
On Thu, Nov 18, 2021 at 03:08:06PM -0800, Tadeusz Struk wrote:
Hi, This has triggered in 5.10.77 yesterday [1], and I was able to reproduce it on 5.10.80 using the C repro from android-54 [2]. What happens is that the function do_mpage_readpage() calls bdev_read_page() [3] passing in bdev == NULL, and bdev_read_page() crashes here [4]. This happens in 5.15 down to 5.10, but it is fixed in 5.16-rc1. I bisected it to the first good commit, which is:
af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size")
The root cause seems to be loss of precision in loop_configure(), when it calls loop_validate_block_size() in [5]. The config->block_size is an uint32 and the bsize param passed to loop_validate_block_size() is unsigned short. The reproducer sets up a loop device with the block size equal to 0x20000400, which is bigger than USHRT_MAX. The loop_validate_block_size() returns 0, but uses the invalid size to setup the device. The new helper changes the bsize param type to uint, and the issue goes away.
To fix this for the older kernels can we please have the two commits:
570b1cac4776 ("block: Add a helper to validate the block size") af3c570fb0df ("loop: Use blk_validate_block_size() to validate block size")
applied to 5.15, 5.14, and 5.10. The first one needs to be back ported, but the second applies cleanly. I will follow up back ports for each version in few minutes.
All now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Damien Le Moal -
Greg KH -
Tadeusz Struk