general protection fault in bdev_read_page

general protection fault in bdev_read_page

[Tadeusz Struk]

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.

-- 
Thanks,
Tadeusz

[1] https://syzkaller.appspot.com/bug?id=2a34ab9dad714959a3d2b60533acbd99094a5c5a
[2] https://syzkaller.appspot.com/x/repro.c?x=13420a05900000
[3] https://elixir.bootlin.com/linux/v5.15/source/fs/mpage.c#L302
[4] https://elixir.bootlin.com/linux/v5.15/source/block/bdev.c#L323
[5] https://elixir.bootlin.com/linux/v5.15/source/drivers/block/loop.c#L1239

[PATCH 5.15] block: Add a helper to validate the block size

[Tadeusz Struk]

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);
 
 /*
-- 
2.33.1

[PATCH 5.14] block: Add a helper to validate the block size

[Tadeusz Struk]

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);
 
 /*
-- 
2.33.1

[PATCH 5.10] block: Add a helper to validate the block size

[Tadeusz Struk]

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);
 
 /*
-- 
2.33.1

Re: [PATCH 5.15] block: Add a helper to validate the block size

[Damien Le Moal]

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 ?

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 5.15] block: Add a helper to validate the block size

[Tadeusz Struk]

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.

-- 
Thanks,
Tadeusz

Re: [PATCH 5.15] block: Add a helper to validate the block size

[Damien Le Moal]

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.


-- 
Damien Le Moal
Western Digital Research

Re: general protection fault in bdev_read_page

[Greg KH]

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