DM-Devel Archive mirror
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org,
	Mikulas Patocka <mpatocka@redhat.com>,
	dm-devel@lists.linux.dev
Subject: Re: block: fail unaligned bio from submit_bio_noacct()
Date: Thu, 21 Mar 2024 11:43:38 -0400	[thread overview]
Message-ID: <ZfxVqkniO-6jFFH5@redhat.com> (raw)
In-Reply-To: <20240321131634.1009972-1-ming.lei@redhat.com>

On Thu, Mar 21 2024 at  9:16P -0400,
Ming Lei <ming.lei@redhat.com> wrote:

> For any bio with data, its start sector and size have to be aligned with
> the queue's logical block size.
> 
> 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.
> 
> Cc: Mikulas Patocka <mpatocka@redhat.com>
> Cc: Mike Snitzer <snitzer@kernel.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  block/blk-core.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index a16b5abdbbf5..b1a10187ef74 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -729,6 +729,20 @@ 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;
> +	unsigned int size = bio->bi_iter.bi_size;
> +
> +	if (size & (bs - 1))
> +		return false;
> +
> +	if (size && ((bio->bi_iter.bi_sector << SECTOR_SHIFT) & (bs - 1)))
> +		return false;
> +
> +	return true;
> +}
> +
>  /**
>   * submit_bio_noacct - re-submit a bio to the block device layer for I/O
>   * @bio:  The bio describing the location in memory and on the device.
> @@ -780,6 +794,9 @@ void submit_bio_noacct(struct bio *bio)
>  		}
>  	}
>  
> +	if (WARN_ON_ONCE(!bio_check_alignment(bio, q)))
> +		goto end_io;
> +
>  	if (!test_bit(QUEUE_FLAG_POLL, &q->queue_flags))
>  		bio_clear_polled(bio);
>  
> -- 
> 2.41.0
> 
> 

This check would really help more quickly find buggy code, but it
would be unfortunate for these extra checks to be required in
production.  It feels like this is the type of check that should be
wrapped by a debug CONFIG option (so only debug kernels have it).

Do we already have an appropriate CONFIG option to use?

Mike

       reply	other threads:[~2024-03-21 15:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240321131634.1009972-1-ming.lei@redhat.com>
2024-03-21 15:43 ` Mike Snitzer [this message]
2024-03-21 17:01   ` block: fail unaligned bio from submit_bio_noacct() Mikulas Patocka
2024-03-21 22:07     ` Christoph Hellwig
2024-03-22  2:08     ` Ming Lei
2024-03-22  2:39       ` Keith Busch
2024-03-24  8:02         ` Ming Lei
2024-03-22 10:16       ` Mikulas Patocka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZfxVqkniO-6jFFH5@redhat.com \
    --to=snitzer@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@lists.linux.dev \
    --cc=linux-block@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=mpatocka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).