2021-06-04 20:02:06

by Satya Tangirala

Subject: [PATCH v3 04/10] block: respect bio_required_sector_alignment() in blk-crypto-fallback

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 <[email protected]>
---
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) {
--
2.32.0.rc1.229.g3e70b5a671-goog

2021-06-17 02:42:25

by Eric Biggers

Subject: Re: [PATCH v3 04/10] block: respect bio_required_sector_alignment() in blk-crypto-fallback

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 <[email protected]>
> ---
> 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 <[email protected]>

2021-06-17 04:35:40

by Eric Biggers

Subject: Re: [PATCH v3 04/10] block: respect bio_required_sector_alignment() in blk-crypto-fallback

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 <[email protected]>
> > ---
> > 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 <[email protected]>

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/[email protected]).
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