Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 16 Jun 2021 21:34:52 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Satya Tangirala <satyat@google.com>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH v3 04/10] block: respect bio_required_sector_alignment()
 in blk-crypto-fallback
Message-ID: <YMrQ7KjD8BpbqNyK@sol.localdomain>
References: <20210604195900.2096121-1-satyat@google.com>
 <20210604195900.2096121-5-satyat@google.com>
 <YMqZuBssXiMvDxIf@sol.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YMqZuBssXiMvDxIf@sol.localdomain>
Precedence: bulk

On Wed, Jun 16, 2021 at 05:39:20PM -0700, Eric Biggers wrote:
> On Fri, Jun 04, 2021 at 07:58:54PM +0000, Satya Tangirala wrote:
> > Make blk_crypto_split_bio_if_needed() respect
> > bio_required_sector_alignment() when calling bio_split(). Without this,
> > blk-crypto-fallback could possibly split a bio in the middle of a data
> > unit, and the resulting bios can no longer be encrypted (since encryption
> > can only be done on complete crypto data units).
> > 
> > Signed-off-by: Satya Tangirala <satyat@google.com>
> > ---
> >  block/blk-crypto-fallback.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> > index c322176a1e09..85c813ef670b 100644
> > --- a/block/blk-crypto-fallback.c
> > +++ b/block/blk-crypto-fallback.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/module.h>
> >  #include <linux/random.h>
> >  
> > +#include "blk.h"
> >  #include "blk-crypto-internal.h"
> >  
> >  static unsigned int num_prealloc_bounce_pg = 32;
> > @@ -225,6 +226,8 @@ static bool blk_crypto_split_bio_if_needed(struct bio **bio_ptr)
> >  	if (num_sectors < bio_sectors(bio)) {
> >  		struct bio *split_bio;
> >  
> > +		num_sectors = round_down(num_sectors,
> > +					 bio_required_sector_alignment(bio));
> >  		split_bio = bio_split(bio, num_sectors, GFP_NOIO,
> >  				      &crypto_bio_split);
> >  		if (!split_bio) {
> > -- 
> 
> Reviewed-by: Eric Biggers <ebiggers@google.com>

Hmm, on second thought, I don't think this patch makes sense without the patch
"block: blk-crypto-fallback: handle data unit split across multiple bvecs"
which Satya sent out in his other series
(https://lkml.kernel.org/r/20210604210908.2105870-2-satyat@google.com).
Either blk-crypto-fallback assumes that the length of every bio_vec is aligned
to data_unit_size (this is the status quo), in which case the round_down() is
unnecessary, *or* it assumes that only the total length is aligned to
data_unit_size, in which case both patches are needed.  So I'm thinking these
should be combined into one patch.

- Eric