Re: [PATCH V2] block: fail unaligned bio from submit_bio_noacct()

[Mike Snitzer <snitzer@kernel.org>]

On Sun, Mar 24 2024 at  9:37P -0400,
Ming Lei <ming.lei@redhat.com> wrote:

> For any FS bio, its start sector and size have to be aligned with the
> queue's logical block size from beginning, because bio split code can't
> make one aligned bio.
> 
> This rule is obvious, but there is still user which may send unaligned
> bio to block layer, and it is observed that dm-integrity can do that,
> and cause double free of driver's dma meta buffer.
> 
> So failfast unaligned bio from submit_bio_noacct() for avoiding more
> troubles.
> 
> Meantime remove this kind of check in dio and discard code path.
> 
> Cc: Keith Busch <kbusch@kernel.org>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Mikulas Patocka <mpatocka@redhat.com>
> Cc: Mike Snitzer <snitzer@kernel.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> V2:
> 	- remove the check in dio and discard code path
> 	- check .bi_sector with (logical_block_size >> 9) - 1
> 
>  block/blk-core.c | 16 ++++++++++++++++
>  block/blk-lib.c  | 17 -----------------
>  block/fops.c     |  3 +--
>  3 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a16b5abdbbf5..2d86922f95e3 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -729,6 +729,19 @@ void submit_bio_noacct_nocheck(struct bio *bio)
>  		__submit_bio_noacct(bio);
>  }
>  
> +static bool bio_check_alignment(struct bio *bio, struct request_queue *q)
> +{
> +	unsigned int bs = q->limits.logical_block_size;
> +
> +	if (bio->bi_iter.bi_size & (bs - 1))
> +		return false;
> +
> +	if (bio->bi_iter.bi_sector & ((bs >> SECTOR_SHIFT) - 1))
> +		return false;
> +
> +	return true;
> +}
> +

You missed Christoph's reply to v1 where he offered:
"This should just use bdev_logical_block_size() on bio->bi_bdev."

Otherwise, looks good.

Mike