Re: [PATCH RFC 3/5] block: validate bio bounds in the queue entered context

On 2026/05/20 9:25, Damien Le Moal wrote:
>> Should this be a pr_err_ratelimited() ?

FWIW, I think pr_info is deliberate and worth keeping.  The comment this
patch removes together with bio_check_eod() gave the reason:

  /*
   * Check whether this bio extends beyond the end of the device or
   * partition.  This may well happen - the kernel calls bread() without
   * checking the size of the device, e.g., when mounting a file system.
   */

So it fires on a legitimate and expected pattern, and pr_err would be
noisy for it.  It may be worth carrying that comment over to the new
location so the reason for the level does not get lost.

Keith, I checked this patch against the nvme_setup_rw() WARN I reported
(the second link in your cover letter).  __bio_split_to_limits() is
called from blk_mq_submit_bio() after bio_queue_enter(), on both the
cached-request path and the newly-allocated-request path, so the bounds
check is now serialized against the freeze window in
nvme_update_ns_info_block().  With the capacity forced to 0, the bio is
failed here and can no longer reach nvme_queue_rq(), which is exactly
the race I hit.  Thanks for picking it up, and sorry for the slow reply.

I'll drop my own "nvme: don't WARN on I/O to a namespace revalidated to
unusable metadata" patch in favour of this.

Reviewed-by: Chao Shi <[email protected]>

Best,
Chao

On Tue, May 19, 2026 at 1:23 PM Keith Busch <[email protected]> wrote:
>
> From: Keith Busch <[email protected]>
>
> bio_check_eod() in submit_bio_noacct() validates that a bio does not
> extend beyond the partition's available sectors. This check runs before
> bio_queue_enter(), so it is not serialized against queue limit updates.
> A driver that freezes the queue, updates limits, changes the capacity,
> and unfreezes can race with a bio that passed the early check under the
> old capacity.
>
> Remove bio_check_eod() and replace it with a bounds check in
> __bio_split_to_limits(), which runs after the queue usage reference has
> been acquired. The check uses partition-aware arithmetic to validate
> both partition bounds and disk capacity in a single comparison that
> works correctly on the post-remap sector values.
>
> Signed-off-by: Keith Busch <[email protected]>
> ---
>  block/blk-core.c | 26 --------------------------
>  block/blk.h      | 14 ++++++++++++++
>  2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 92a802dc8042c..c200d0fc44fe7 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -547,30 +547,6 @@ int should_fail_bio(struct bio *bio)
>  }
>  ALLOW_ERROR_INJECTION(should_fail_bio, ERRNO);
>
> -/*
> - * Check whether this bio extends beyond the end of the device or partition.
> - * This may well happen - the kernel calls bread() without checking the size of
> - * the device, e.g., when mounting a file system.
> - */
> -static inline int bio_check_eod(struct bio *bio)
> -{
> -       sector_t maxsector = bdev_nr_sectors(bio->bi_bdev);
> -       unsigned int nr_sectors = bio_sectors(bio);
> -
> -       if (nr_sectors &&
> -           (nr_sectors > maxsector ||
> -            bio->bi_iter.bi_sector > maxsector - nr_sectors)) {
> -               if (!maxsector)
> -                       return -EIO;
> -               pr_info_ratelimited("%s: attempt to access beyond end of device\n"
> -                                   "%pg: rw=%d, sector=%llu, nr_sectors = %u limit=%llu\n",
> -                                   current->comm, bio->bi_bdev, bio->bi_opf,
> -                                   bio->bi_iter.bi_sector, nr_sectors, maxsector);
> -               return -EIO;
> -       }
> -       return 0;
> -}
> -
>  /*
>   * Remap block n of partition p to block n+start(p) of the disk.
>   */
> @@ -802,8 +778,6 @@ void submit_bio_noacct(struct bio *bio)
>                 goto end_io;
>         bio_check_ro(bio);
>         if (!bio_flagged(bio, BIO_REMAPPED)) {
> -               if (unlikely(bio_check_eod(bio)))
> -                       goto end_io;
>                 if (bdev_is_partition(bdev) &&
>                     unlikely(blk_partition_remap(bio)))
>                         goto end_io;
> diff --git a/block/blk.h b/block/blk.h
> index bf1a80493ff1c..e70acb2d358e3 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -423,6 +423,17 @@ static inline bool bio_may_need_split(struct bio *bio,
>  static inline struct bio *__bio_split_to_limits(struct bio *bio,
>                 const struct queue_limits *lim, unsigned int *nr_segs)
>  {
> +       if (unlikely(bio_end_sector(bio) > bdev_nr_sectors(bio->bi_bdev) +
> +                                          bio->bi_bdev->bd_start_sect)) {
> +               pr_info_ratelimited("%s: attempt to access beyond end of device\n"
> +                                   "%pg: rw=%d, sector=%llu, nr_sectors = %u limit=%llu\n",
> +                                   current->comm, bio->bi_bdev, bio->bi_opf,
> +                                   bio->bi_iter.bi_sector, bio_sectors(bio),
> +                                   bdev_nr_sectors(bio->bi_bdev) +
> +                                       bio->bi_bdev->bd_start_sect);
> +               goto ioerr;
> +       }
> +
>         switch (bio_op(bio)) {
>         case REQ_OP_READ:
>         case REQ_OP_WRITE:
> @@ -442,6 +453,9 @@ static inline struct bio *__bio_split_to_limits(struct bio *bio,
>                 *nr_segs = 0;
>                 return bio;
>         }
> +ioerr:
> +       bio_io_error(bio);
> +       return NULL;
>  }
>
>  /**
> --
> 2.53.0-Meta
>