linux-kernel - Re: [PATCH v2 4/8] block: introduce bio_required_sector

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 15 Apr 2021 12:37:44 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Satya Tangirala <satyat@...gle.com>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH v2 4/8] block: introduce bio_required_sector_alignment()

On Tue, Mar 30, 2021 at 07:06:53PM +0100, Christoph Hellwig wrote:
> On Thu, Mar 25, 2021 at 09:26:05PM +0000, Satya Tangirala wrote:
> > +/*
> > + * The required sector alignment for a bio. The number of sectors in any bio
> > + * that's constructed/split must be aligned to this value.
> > + */
> > +static inline unsigned int bio_required_sector_alignment(struct bio *bio)
> > +{
> > +	struct request_queue *q = bio->bi_bdev->bd_disk->queue;
> > +
> > +	return max(queue_logical_block_size(q) >> SECTOR_SHIFT,
> > +		   blk_crypto_bio_sectors_alignment(bio));
> > +}
> 
> It might make more sense to just have a field in the request queue
> for the max alignment so that the fast path just looks at one field.
> Then the various setup time functions would update it to the maximum
> required.

I don't see how that would work here, as the required alignment is a per-bio
thing which depends on whether the bio has an encryption context or not, and (if
it does) also the data_unit_size the submitter of the bio selected.

We could just always assume the worst-case scenario, but that seems
unnecessarily pessimistic?

- Eric