diff mbox series

xen: pvblock: fix the maximum io size in one operation

Message ID 20231115015345.490313-1-takahiro.akashi@linaro.org
State Accepted
Commit 654580eee13bc7a0d4ed4cad2b2fead1ec88107a
Headers show
Series xen: pvblock: fix the maximum io size in one operation | expand

Commit Message

AKASHI Takahiro Nov. 15, 2023, 1:53 a.m. UTC
The current implementation may cause BUG_ON() in blkfront_aio()
	BUG_ON(n > BLKIF_MAX_SEGMENTS_PER_REQUEST);

In pvblock_iop(), a read/write operation will be split into smaller
chunks of data so that the size in one access (aio_nbytes) is limited
to, at the maximum,
	BLKIF_MAX_SEGMENTS_PER_REQUEST * PAGE_SIZE

But this works only if when the *buffer* passed in to pvblock_io()
is page-aligned. If not, the given data region may stand across
(BLKIF_MAX_SEGMENTS_PER_REQUEST + 1) pages. See the logic in
blkfront_aio():
	start = (uintptr_t)aiocbp->aio_buf & PAGE_MASK;
	end = ((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes +
	       PAGE_SIZE - 1) & PAGE_MASK;
Then this will lead to BUG_ON() above.

This can be fixed by decreasing the maximum size of aio_nbytes.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Fixes: commit 3a739cc6c948 ("xen: pvblock: Implement front-back protocol and do IO")
---
 drivers/xen/pvblock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Nastya Vicodin Nov. 16, 2023, 7:43 a.m. UTC | #1
Reviewed-by <Anastasiia Lukianenko vicooodin@gmail.com>

Regards,
Anastasiia

On Wed, Nov 15, 2023 at 2:53 AM AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> The current implementation may cause BUG_ON() in blkfront_aio()
>         BUG_ON(n > BLKIF_MAX_SEGMENTS_PER_REQUEST);
>
> In pvblock_iop(), a read/write operation will be split into smaller
> chunks of data so that the size in one access (aio_nbytes) is limited
> to, at the maximum,
>         BLKIF_MAX_SEGMENTS_PER_REQUEST * PAGE_SIZE
>
> But this works only if when the *buffer* passed in to pvblock_io()
> is page-aligned. If not, the given data region may stand across
> (BLKIF_MAX_SEGMENTS_PER_REQUEST + 1) pages. See the logic in
> blkfront_aio():
>         start = (uintptr_t)aiocbp->aio_buf & PAGE_MASK;
>         end = ((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes +
>                PAGE_SIZE - 1) & PAGE_MASK;
> Then this will lead to BUG_ON() above.
>
> This can be fixed by decreasing the maximum size of aio_nbytes.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Fixes: commit 3a739cc6c948 ("xen: pvblock: Implement front-back protocol
> and do IO")
> ---
>  drivers/xen/pvblock.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/pvblock.c b/drivers/xen/pvblock.c
> index 4ad548d599d5..1df04e239ad0 100644
> --- a/drivers/xen/pvblock.c
> +++ b/drivers/xen/pvblock.c
> @@ -632,7 +632,8 @@ static ulong pvblock_iop(struct udevice *udev,
> lbaint_t blknr,
>                         memcpy(blk_dev->bounce_buffer, buffer,
> desc->blksz);
>
>                 aiocb.aio_nbytes = unaligned ? desc->blksz :
> -                       min((size_t)(BLKIF_MAX_SEGMENTS_PER_REQUEST *
> PAGE_SIZE),
> +                       min((size_t)((BLKIF_MAX_SEGMENTS_PER_REQUEST - 1)
> +                                       * PAGE_SIZE),
>                             (size_t)(blocks_todo * desc->blksz));
>
>                 blkfront_io(&aiocb, write);
> --
> 2.34.1
>
>
Tom Rini Nov. 29, 2023, 4:58 p.m. UTC | #2
On Wed, Nov 15, 2023 at 10:53:45AM +0900, AKASHI Takahiro wrote:

> The current implementation may cause BUG_ON() in blkfront_aio()
> 	BUG_ON(n > BLKIF_MAX_SEGMENTS_PER_REQUEST);
> 
> In pvblock_iop(), a read/write operation will be split into smaller
> chunks of data so that the size in one access (aio_nbytes) is limited
> to, at the maximum,
> 	BLKIF_MAX_SEGMENTS_PER_REQUEST * PAGE_SIZE
> 
> But this works only if when the *buffer* passed in to pvblock_io()
> is page-aligned. If not, the given data region may stand across
> (BLKIF_MAX_SEGMENTS_PER_REQUEST + 1) pages. See the logic in
> blkfront_aio():
> 	start = (uintptr_t)aiocbp->aio_buf & PAGE_MASK;
> 	end = ((uintptr_t)aiocbp->aio_buf + aiocbp->aio_nbytes +
> 	       PAGE_SIZE - 1) & PAGE_MASK;
> Then this will lead to BUG_ON() above.
> 
> This can be fixed by decreasing the maximum size of aio_nbytes.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Fixes: commit 3a739cc6c948 ("xen: pvblock: Implement front-back protocol and do IO")

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/drivers/xen/pvblock.c b/drivers/xen/pvblock.c
index 4ad548d599d5..1df04e239ad0 100644
--- a/drivers/xen/pvblock.c
+++ b/drivers/xen/pvblock.c
@@ -632,7 +632,8 @@  static ulong pvblock_iop(struct udevice *udev, lbaint_t blknr,
 			memcpy(blk_dev->bounce_buffer, buffer, desc->blksz);
 
 		aiocb.aio_nbytes = unaligned ? desc->blksz :
-			min((size_t)(BLKIF_MAX_SEGMENTS_PER_REQUEST * PAGE_SIZE),
+			min((size_t)((BLKIF_MAX_SEGMENTS_PER_REQUEST - 1)
+					* PAGE_SIZE),
 			    (size_t)(blocks_todo * desc->blksz));
 
 		blkfront_io(&aiocb, write);