2021-01-21 23:06:11

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 0/8] add support for direct I/O with fscrypt using blk-crypto

This patch series adds support for direct I/O with fscrypt using
blk-crypto.

Till now, the blk-crypto-fallback expected the offset and length of each
bvec in a bio to be aligned to the crypto data unit size. This in turn
would mean that any user buffer used to read/write encrypted data using the
blk-crypto framework would need to be aligned to the crypto data unit size.
Patch 1 enables blk-crypto-fallback to work without this requirement. It
also relaxes the alignment requirement that blk-crypto checks for - now,
blk-crypto only requires that the length of the I/O is aligned to the
crypto data unit size. This allows direct I/O support introduced in the
later patches in this series to require extra alignment restrictions on
user buffers.

Patch 2 relaxes the alignment check that blk-crypto performs on bios.
blk-crypto would check that the offset and length of each bvec in a bio is
aligned to the data unit size, since the blk-crypto-fallback required it.
As this is no longer the case, blk-crypto now only checks that the total
length of the bio is data unit size aligned.

Patch 3 adds two functions to fscrypt that need to be called to determine
if direct I/O is supported for a request.

Patches 4 and 5 modify direct-io and iomap respectively to set bio crypt
contexts on bios when appropriate by calling into fscrypt.

Patches 6 and 7 allow ext4 and f2fs direct I/O to support fscrypt without
falling back to buffered I/O.

Patch 8 updates the fscrypt documentation for direct I/O support.
The documentation now notes the required conditions for inline encryption
and direct I/O on encrypted files.

This patch series was tested by running xfstests with test_dummy_encryption
with and without the 'inlinecrypt' mount option, and there were no
meaningful regressions. Without any modification, xfstests skip any
direct I/O test when using ext4/encrypt and f2fs/encrypt, so I modified
xfstests not to skip those tests.

Among those tests, generic/465 fails with ext4/encrypt because a bio ends
up being split in the middle of a crypto data unit. Patch 1 from v7 (which
has been sent out as a separate patch series) fixes this.

Note that the blk-crypto-fallback changes (Patch 1 in v8 in this series)
were also tested through xfstests by using this series along with the patch
series that ensures bios aren't split in the middle of a data unit (Patch 1
from v7) - Some tests (such as generic/465 again) result in bvecs that
don't contain a complete data unit (so a data unit is split across multiple
bvecs), and only pass with this patch.

Changes v7 => v8:
- Patch 1 from v7 (which ensured that bios aren't split in the middle of
a data unit) has been sent out in a separate patch series, as it's
required even without this patch series. That patch series can now
be found at
https://lore.kernel.org/linux-block/[email protected]/
- Patch 2 from v7 has been split into 2 patches (Patch 1 and 2 in v8).
- Update docs

Changes v6 => v7:
- add patches 1 and 2 to allow blk-crypto to work with user buffers not
aligned to crypto data unit size, so that direct I/O doesn't require
that alignment either.
- some cleanups

Changes v5 => v6:
- fix bug with fscrypt_limit_io_blocks() and make it ready for 64 bit
block numbers.
- remove Reviewed-by for Patch 1 due to significant changes from when
the Reviewed-by was given.

Changes v4 => v5:
- replace fscrypt_limit_io_pages() with fscrypt_limit_io_block(), which
is now called by individual filesystems (currently only ext4) instead
of the iomap code. This new function serves the same end purpose as
the one it replaces (ensuring that DUNs within a bio are contiguous)
but operates purely with blocks instead of with pages.
- make iomap_dio_zero() set bio_crypt_ctx's again, instead of just a
WARN_ON() since some folks prefer that instead.
- add Reviewed-by's

Changes v3 => v4:
- Fix bug in iomap_dio_bio_actor() where fscrypt_limit_io_pages() was
being called too early (thanks Eric!)
- Improve comments and fix formatting in documentation
- iomap_dio_zero() is only called to zero out partial blocks, but
direct I/O is only supported on encrypted files when I/O is
blocksize aligned, so it doesn't need to set encryption contexts on
bios. Replace setting the encryption context with a WARN_ON(). (Eric)

Changes v2 => v3:
- add changelog to coverletter

Changes v1 => v2:
- Fix bug in f2fs caused by replacing f2fs_post_read_required() with
!fscrypt_dio_supported() since the latter doesn't check for
compressed inodes unlike the former.
- Add patches 6 and 7 for fscrypt documentation
- cleanups and comments

Eric Biggers (5):
fscrypt: add functions for direct I/O support
direct-io: add support for fscrypt using blk-crypto
iomap: support direct I/O with fscrypt using blk-crypto
ext4: support direct I/O with fscrypt using blk-crypto
f2fs: support direct I/O with fscrypt using blk-crypto

Satya Tangirala (3):
block: blk-crypto-fallback: handle data unit split across multiple
bvecs
block: blk-crypto: relax alignment requirements for bvecs in bios
fscrypt: update documentation for direct I/O support

Documentation/filesystems/fscrypt.rst | 21 ++-
block/blk-crypto-fallback.c | 203 ++++++++++++++++++++------
block/blk-crypto.c | 19 +--
fs/crypto/crypto.c | 8 +
fs/crypto/inline_crypt.c | 74 ++++++++++
fs/direct-io.c | 15 +-
fs/ext4/file.c | 10 +-
fs/ext4/inode.c | 7 +
fs/f2fs/f2fs.h | 6 +-
fs/iomap/direct-io.c | 6 +
include/linux/fscrypt.h | 18 +++
11 files changed, 315 insertions(+), 72 deletions(-)

--
2.30.0.280.ga3ce27912f-goog


2021-01-21 23:26:18

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 8/8] fscrypt: update documentation for direct I/O support

Update fscrypt documentation to reflect the addition of direct I/O support
and document the necessary conditions for direct I/O on encrypted files.

Signed-off-by: Satya Tangirala <[email protected]>
Reviewed-by: Eric Biggers <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
---
Documentation/filesystems/fscrypt.rst | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 44b67ebd6e40..c0c1747fa2fb 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -1047,8 +1047,10 @@ astute users may notice some differences in behavior:
may be used to overwrite the source files but isn't guaranteed to be
effective on all filesystems and storage devices.

-- Direct I/O is not supported on encrypted files. Attempts to use
- direct I/O on such files will fall back to buffered I/O.
+- Direct I/O is supported on encrypted files only under some
+ circumstances (see `Direct I/O support`_ for details). When these
+ circumstances are not met, attempts to use direct I/O on encrypted
+ files will fall back to buffered I/O.

- The fallocate operations FALLOC_FL_COLLAPSE_RANGE and
FALLOC_FL_INSERT_RANGE are not supported on encrypted files and will
@@ -1121,6 +1123,21 @@ It is not currently possible to backup and restore encrypted files
without the encryption key. This would require special APIs which
have not yet been implemented.

+Direct I/O support
+==================
+
+Direct I/O on encrypted files is supported through blk-crypto. In
+particular, this means the kernel must have CONFIG_BLK_INLINE_ENCRYPTION
+enabled, the filesystem must have had the 'inlinecrypt' mount option
+specified, and either hardware inline encryption must be present, or
+CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK must have been enabled. Further,
+the starting position in the file and the length of any I/O must be aligned
+to the filesystem block size (*not* necessarily the same as the block
+device's block size). If any of these conditions isn't met, attempts to do
+direct I/O on an encrypted file will fall back to buffered I/O. However,
+there aren't any additional requirements on user buffer alignment (apart
+from those already present when using direct I/O on unencrypted files).
+
Encryption policy enforcement
=============================

--
2.30.0.280.ga3ce27912f-goog

2021-01-21 23:27:32

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 6/8] ext4: support direct I/O with fscrypt using blk-crypto

From: Eric Biggers <[email protected]>

Wire up ext4 with fscrypt direct I/O support. Direct I/O with fscrypt is
only supported through blk-crypto (i.e. CONFIG_BLK_INLINE_ENCRYPTION must
have been enabled, the 'inlinecrypt' mount option must have been specified,
and either hardware inline encryption support must be present or
CONFIG_BLK_INLINE_ENCYRPTION_FALLBACK must have been enabled). Further,
direct I/O on encrypted files is only supported when the *length* of the
I/O is aligned to the filesystem block size (which is *not* necessarily the
same as the block device's block size).

fscrypt_limit_io_blocks() is called before setting up the iomap to ensure
that the blocks of each bio that iomap will submit will have contiguous
DUNs. Note that fscrypt_limit_io_blocks() is normally a no-op, as normally
the DUNs simply increment along with the logical blocks. But it's needed
to handle an edge case in one of the fscrypt IV generation methods.

Signed-off-by: Eric Biggers <[email protected]>
Co-developed-by: Satya Tangirala <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
Acked-by: Theodore Ts'o <[email protected]>
---
fs/ext4/file.c | 10 ++++++----
fs/ext4/inode.c | 7 +++++++
2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 349b27f0dda0..77681ba5e6cc 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -36,9 +36,11 @@
#include "acl.h"
#include "truncate.h"

-static bool ext4_dio_supported(struct inode *inode)
+static bool ext4_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
{
- if (IS_ENABLED(CONFIG_FS_ENCRYPTION) && IS_ENCRYPTED(inode))
+ struct inode *inode = file_inode(iocb->ki_filp);
+
+ if (!fscrypt_dio_supported(iocb, iter))
return false;
if (fsverity_active(inode))
return false;
@@ -61,7 +63,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
inode_lock_shared(inode);
}

- if (!ext4_dio_supported(inode)) {
+ if (!ext4_dio_supported(iocb, to)) {
inode_unlock_shared(inode);
/*
* Fallback to buffered I/O if the operation being performed on
@@ -495,7 +497,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
}

/* Fallback to buffered I/O if the inode does not support direct I/O. */
- if (!ext4_dio_supported(inode)) {
+ if (!ext4_dio_supported(iocb, from)) {
if (ilock_shared)
inode_unlock_shared(inode);
else
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c173c8405856..e5407699ce92 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3482,6 +3482,13 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
if (ret < 0)
return ret;
out:
+ /*
+ * When inline encryption is enabled, sometimes I/O to an encrypted file
+ * has to be broken up to guarantee DUN contiguity. Handle this by
+ * limiting the length of the mapping returned.
+ */
+ map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
ext4_set_iomap(inode, iomap, &map, offset, length);

return 0;
--
2.30.0.280.ga3ce27912f-goog

2021-01-21 23:27:33

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 1/8] block: blk-crypto-fallback: handle data unit split across multiple bvecs

Till now, the blk-crypto-fallback required each crypto data unit to be
contained within a single bvec. It also required the starting offset
of each bvec to be aligned to the data unit size. This patch removes
both restrictions, so that blk-crypto-fallback can handle crypto data
units split across multiple bvecs. blk-crypto-fallback now only requires
that the total size of the bio be aligned to the crypto data unit size.
The buffer that is being read/written to no longer needs to be data unit
size aligned.

This is useful for making the alignment requirements for direct I/O on
encrypted files similar to those for direct I/O on unencrypted files.

Co-developed-by: Eric Biggers <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
---
block/blk-crypto-fallback.c | 203 +++++++++++++++++++++++++++---------
1 file changed, 156 insertions(+), 47 deletions(-)

diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index c162b754efbd..663579d0783f 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -249,6 +249,65 @@ static void blk_crypto_dun_to_iv(const u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE],
iv->dun[i] = cpu_to_le64(dun[i]);
}

+/*
+ * If the length of any bio segment isn't a multiple of data_unit_size
+ * (which can happen if data_unit_size > logical_block_size), then each
+ * encryption/decryption might need to be passed multiple scatterlist elements.
+ * If that will be the case, this function allocates and initializes src and dst
+ * scatterlists (or a combined src/dst scatterlist) with the needed length.
+ *
+ * If 1 element is guaranteed to be enough (which is usually the case, and is
+ * guaranteed when data_unit_size <= logical_block_size), then this function
+ * just initializes the on-stack scatterlist(s).
+ */
+static bool blk_crypto_alloc_sglists(struct bio *bio,
+ const struct bvec_iter *start_iter,
+ unsigned int data_unit_size,
+ struct scatterlist **src_p,
+ struct scatterlist **dst_p)
+{
+ struct bio_vec bv;
+ struct bvec_iter iter;
+ bool aligned = true;
+ unsigned int count = 0;
+
+ __bio_for_each_segment(bv, bio, iter, *start_iter) {
+ count++;
+ aligned &= IS_ALIGNED(bv.bv_len, data_unit_size);
+ }
+ if (aligned) {
+ count = 1;
+ } else {
+ /*
+ * We can't need more elements than bio segments, and we can't
+ * need more than the number of sectors per data unit. This may
+ * overestimate the required length by a bit, but that's okay.
+ */
+ count = min(count, data_unit_size >> SECTOR_SHIFT);
+ }
+
+ if (count > 1) {
+ *src_p = kmalloc_array(count, sizeof(struct scatterlist),
+ GFP_NOIO);
+ if (!*src_p)
+ return false;
+ if (dst_p) {
+ *dst_p = kmalloc_array(count,
+ sizeof(struct scatterlist),
+ GFP_NOIO);
+ if (!*dst_p) {
+ kfree(*src_p);
+ *src_p = NULL;
+ return false;
+ }
+ }
+ }
+ sg_init_table(*src_p, count);
+ if (dst_p)
+ sg_init_table(*dst_p, count);
+ return true;
+}
+
/*
* The crypto API fallback's encryption routine.
* Allocate a bounce bio for encryption, encrypt the input bio using crypto API,
@@ -265,9 +324,12 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
struct skcipher_request *ciph_req = NULL;
DECLARE_CRYPTO_WAIT(wait);
u64 curr_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
- struct scatterlist src, dst;
+ struct scatterlist _src, *src = &_src;
+ struct scatterlist _dst, *dst = &_dst;
union blk_crypto_iv iv;
- unsigned int i, j;
+ unsigned int i;
+ unsigned int sg_idx = 0;
+ unsigned int du_filled = 0;
bool ret = false;
blk_status_t blk_st;

@@ -279,11 +341,18 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
bc = src_bio->bi_crypt_context;
data_unit_size = bc->bc_key->crypto_cfg.data_unit_size;

+ /* Allocate scatterlists if needed */
+ if (!blk_crypto_alloc_sglists(src_bio, &src_bio->bi_iter,
+ data_unit_size, &src, &dst)) {
+ src_bio->bi_status = BLK_STS_RESOURCE;
+ return false;
+ }
+
/* Allocate bounce bio for encryption */
enc_bio = blk_crypto_clone_bio(src_bio);
if (!enc_bio) {
src_bio->bi_status = BLK_STS_RESOURCE;
- return false;
+ goto out_free_sglists;
}

/*
@@ -303,45 +372,58 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
}

memcpy(curr_dun, bc->bc_dun, sizeof(curr_dun));
- sg_init_table(&src, 1);
- sg_init_table(&dst, 1);

- skcipher_request_set_crypt(ciph_req, &src, &dst, data_unit_size,
+ skcipher_request_set_crypt(ciph_req, src, dst, data_unit_size,
iv.bytes);

- /* Encrypt each page in the bounce bio */
+ /*
+ * Encrypt each data unit in the bounce bio.
+ *
+ * Take care to handle the case where a data unit spans bio segments.
+ * This can happen when data_unit_size > logical_block_size.
+ */
for (i = 0; i < enc_bio->bi_vcnt; i++) {
- struct bio_vec *enc_bvec = &enc_bio->bi_io_vec[i];
- struct page *plaintext_page = enc_bvec->bv_page;
+ struct bio_vec *bv = &enc_bio->bi_io_vec[i];
+ struct page *plaintext_page = bv->bv_page;
struct page *ciphertext_page =
mempool_alloc(blk_crypto_bounce_page_pool, GFP_NOIO);
+ unsigned int offset_in_bv = 0;

- enc_bvec->bv_page = ciphertext_page;
+ bv->bv_page = ciphertext_page;

if (!ciphertext_page) {
src_bio->bi_status = BLK_STS_RESOURCE;
goto out_free_bounce_pages;
}

- sg_set_page(&src, plaintext_page, data_unit_size,
- enc_bvec->bv_offset);
- sg_set_page(&dst, ciphertext_page, data_unit_size,
- enc_bvec->bv_offset);
-
- /* Encrypt each data unit in this page */
- for (j = 0; j < enc_bvec->bv_len; j += data_unit_size) {
- blk_crypto_dun_to_iv(curr_dun, &iv);
- if (crypto_wait_req(crypto_skcipher_encrypt(ciph_req),
- &wait)) {
- i++;
- src_bio->bi_status = BLK_STS_IOERR;
- goto out_free_bounce_pages;
+ while (offset_in_bv < bv->bv_len) {
+ unsigned int n = min(bv->bv_len - offset_in_bv,
+ data_unit_size - du_filled);
+ sg_set_page(&src[sg_idx], plaintext_page, n,
+ bv->bv_offset + offset_in_bv);
+ sg_set_page(&dst[sg_idx], ciphertext_page, n,
+ bv->bv_offset + offset_in_bv);
+ sg_idx++;
+ offset_in_bv += n;
+ du_filled += n;
+ if (du_filled == data_unit_size) {
+ blk_crypto_dun_to_iv(curr_dun, &iv);
+ if (crypto_wait_req(crypto_skcipher_encrypt(ciph_req),
+ &wait)) {
+ src_bio->bi_status = BLK_STS_IOERR;
+ i++;
+ goto out_free_bounce_pages;
+ }
+ bio_crypt_dun_increment(curr_dun, 1);
+ sg_idx = 0;
+ du_filled = 0;
}
- bio_crypt_dun_increment(curr_dun, 1);
- src.offset += data_unit_size;
- dst.offset += data_unit_size;
}
}
+ if (WARN_ON_ONCE(du_filled != 0)) {
+ src_bio->bi_status = BLK_STS_IOERR;
+ goto out_free_bounce_pages;
+ }

enc_bio->bi_private = src_bio;
enc_bio->bi_end_io = blk_crypto_fallback_encrypt_endio;
@@ -362,7 +444,11 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
out_put_enc_bio:
if (enc_bio)
bio_put(enc_bio);
-
+out_free_sglists:
+ if (src != &_src)
+ kfree(src);
+ if (dst != &_dst)
+ kfree(dst);
return ret;
}

@@ -381,13 +467,21 @@ static void blk_crypto_fallback_decrypt_bio(struct work_struct *work)
DECLARE_CRYPTO_WAIT(wait);
u64 curr_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
union blk_crypto_iv iv;
- struct scatterlist sg;
+ struct scatterlist _sg, *sg = &_sg;
struct bio_vec bv;
struct bvec_iter iter;
const int data_unit_size = bc->bc_key->crypto_cfg.data_unit_size;
- unsigned int i;
+ unsigned int sg_idx = 0;
+ unsigned int du_filled = 0;
blk_status_t blk_st;

+ /* Allocate scatterlist if needed */
+ if (!blk_crypto_alloc_sglists(bio, &f_ctx->crypt_iter, data_unit_size,
+ &sg, NULL)) {
+ bio->bi_status = BLK_STS_RESOURCE;
+ goto out_no_sglists;
+ }
+
/*
* Use the crypto API fallback keyslot manager to get a crypto_skcipher
* for the algorithm and key specified for this bio.
@@ -405,33 +499,48 @@ static void blk_crypto_fallback_decrypt_bio(struct work_struct *work)
}

memcpy(curr_dun, bc->bc_dun, sizeof(curr_dun));
- sg_init_table(&sg, 1);
- skcipher_request_set_crypt(ciph_req, &sg, &sg, data_unit_size,
- iv.bytes);
+ skcipher_request_set_crypt(ciph_req, sg, sg, data_unit_size, iv.bytes);

- /* Decrypt each segment in the bio */
+ /*
+ * Decrypt each data unit in the bio.
+ *
+ * Take care to handle the case where a data unit spans bio segments.
+ * This can happen when data_unit_size > logical_block_size.
+ */
__bio_for_each_segment(bv, bio, iter, f_ctx->crypt_iter) {
- struct page *page = bv.bv_page;
-
- sg_set_page(&sg, page, data_unit_size, bv.bv_offset);
-
- /* Decrypt each data unit in the segment */
- for (i = 0; i < bv.bv_len; i += data_unit_size) {
- blk_crypto_dun_to_iv(curr_dun, &iv);
- if (crypto_wait_req(crypto_skcipher_decrypt(ciph_req),
- &wait)) {
- bio->bi_status = BLK_STS_IOERR;
- goto out;
+ unsigned int offset_in_bv = 0;
+
+ while (offset_in_bv < bv.bv_len) {
+ unsigned int n = min(bv.bv_len - offset_in_bv,
+ data_unit_size - du_filled);
+ sg_set_page(&sg[sg_idx++], bv.bv_page, n,
+ bv.bv_offset + offset_in_bv);
+ offset_in_bv += n;
+ du_filled += n;
+ if (du_filled == data_unit_size) {
+ blk_crypto_dun_to_iv(curr_dun, &iv);
+ if (crypto_wait_req(crypto_skcipher_decrypt(ciph_req),
+ &wait)) {
+ bio->bi_status = BLK_STS_IOERR;
+ goto out;
+ }
+ bio_crypt_dun_increment(curr_dun, 1);
+ sg_idx = 0;
+ du_filled = 0;
}
- bio_crypt_dun_increment(curr_dun, 1);
- sg.offset += data_unit_size;
}
}
-
+ if (WARN_ON_ONCE(du_filled != 0)) {
+ bio->bi_status = BLK_STS_IOERR;
+ goto out;
+ }
out:
skcipher_request_free(ciph_req);
blk_ksm_put_slot(slot);
out_no_keyslot:
+ if (sg != &_sg)
+ kfree(sg);
+out_no_sglists:
mempool_free(f_ctx, bio_fallback_crypt_ctx_pool);
bio_endio(bio);
}
--
2.30.0.280.ga3ce27912f-goog

2021-01-21 23:29:17

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 3/8] fscrypt: add functions for direct I/O support

From: Eric Biggers <[email protected]>

Introduce fscrypt_dio_supported() to check whether a direct I/O request
is unsupported due to encryption constraints.

Also introduce fscrypt_limit_io_blocks() to limit how many blocks can be
added to a bio being prepared for direct I/O. This is needed for
filesystems that use the iomap direct I/O implementation to avoid DUN
wraparound in the middle of a bio (which is possible with the
IV_INO_LBLK_32 IV generation method). Elsewhere fscrypt_mergeable_bio()
is used for this, but iomap operates on logical ranges directly, so
filesystems using iomap won't have a chance to call fscrypt_mergeable_bio()
on every block added to a bio. So we need this function which limits a
logical range in one go.

Signed-off-by: Eric Biggers <[email protected]>
Co-developed-by: Satya Tangirala <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
---
fs/crypto/crypto.c | 8 +++++
fs/crypto/inline_crypt.c | 74 ++++++++++++++++++++++++++++++++++++++++
include/linux/fscrypt.h | 18 ++++++++++
3 files changed, 100 insertions(+)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4ef3f714046a..4fcca79f39ae 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -69,6 +69,14 @@ void fscrypt_free_bounce_page(struct page *bounce_page)
}
EXPORT_SYMBOL(fscrypt_free_bounce_page);

+/*
+ * Generate the IV for the given logical block number within the given file.
+ * For filenames encryption, lblk_num == 0.
+ *
+ * Keep this in sync with fscrypt_limit_io_blocks(). fscrypt_limit_io_blocks()
+ * needs to know about any IV generation methods where the low bits of IV don't
+ * simply contain the lblk_num (e.g., IV_INO_LBLK_32).
+ */
void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
const struct fscrypt_info *ci)
{
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index c57bebfa48fe..956f5bfab7a0 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -17,6 +17,7 @@
#include <linux/buffer_head.h>
#include <linux/sched/mm.h>
#include <linux/slab.h>
+#include <linux/uio.h>

#include "fscrypt_private.h"

@@ -363,3 +364,76 @@ bool fscrypt_mergeable_bio_bh(struct bio *bio,
return fscrypt_mergeable_bio(bio, inode, next_lblk);
}
EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
+
+/**
+ * fscrypt_dio_supported() - check whether a direct I/O request is unsupported
+ * due to encryption constraints
+ * @iocb: the file and position the I/O is targeting
+ * @iter: the I/O data segment(s)
+ *
+ * Return: true if direct I/O is supported
+ */
+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
+{
+ const struct inode *inode = file_inode(iocb->ki_filp);
+ const unsigned int blocksize = i_blocksize(inode);
+
+ /* If the file is unencrypted, no veto from us. */
+ if (!fscrypt_needs_contents_encryption(inode))
+ return true;
+
+ /* We only support direct I/O with inline crypto, not fs-layer crypto */
+ if (!fscrypt_inode_uses_inline_crypto(inode))
+ return false;
+
+ /*
+ * Since the granularity of encryption is filesystem blocks, the I/O
+ * must be block aligned -- not just disk sector aligned.
+ */
+ if (!IS_ALIGNED(iocb->ki_pos | iov_iter_count(iter), blocksize))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(fscrypt_dio_supported);
+
+/**
+ * fscrypt_limit_io_blocks() - limit I/O blocks to avoid discontiguous DUNs
+ * @inode: the file on which I/O is being done
+ * @lblk: the block at which the I/O is being started from
+ * @nr_blocks: the number of blocks we want to submit starting at @lblk
+ *
+ * Determine the limit to the number of blocks that can be submitted in the bio
+ * targeting @lblk without causing a data unit number (DUN) discontinuity.
+ *
+ * This is normally just @nr_blocks, as normally the DUNs just increment along
+ * with the logical blocks. (Or the file is not encrypted.)
+ *
+ * In rare cases, fscrypt can be using an IV generation method that allows the
+ * DUN to wrap around within logically continuous blocks, and that wraparound
+ * will occur. If this happens, a value less than @nr_blocks will be returned
+ * so that the wraparound doesn't occur in the middle of the bio.
+ *
+ * Return: the actual number of blocks that can be submitted
+ */
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
+{
+ const struct fscrypt_info *ci = inode->i_crypt_info;
+ u32 dun;
+
+ if (!fscrypt_inode_uses_inline_crypto(inode))
+ return nr_blocks;
+
+ if (nr_blocks <= 1)
+ return nr_blocks;
+
+ if (!(fscrypt_policy_flags(&ci->ci_policy) &
+ FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
+ return nr_blocks;
+
+ /* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
+
+ dun = ci->ci_hashed_ino + lblk;
+
+ return min_t(u64, nr_blocks, (u64)U32_MAX + 1 - dun);
+}
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 2ea1387bb497..d8dde02aee82 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -609,6 +609,10 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
bool fscrypt_mergeable_bio_bh(struct bio *bio,
const struct buffer_head *next_bh);

+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter);
+
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks);
+
#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */

static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
@@ -637,6 +641,20 @@ static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
{
return true;
}
+
+static inline bool fscrypt_dio_supported(struct kiocb *iocb,
+ struct iov_iter *iter)
+{
+ const struct inode *inode = file_inode(iocb->ki_filp);
+
+ return !fscrypt_needs_contents_encryption(inode);
+}
+
+static inline u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk,
+ u64 nr_blocks)
+{
+ return nr_blocks;
+}
#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */

/**
--
2.30.0.280.ga3ce27912f-goog

2021-01-21 23:30:06

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 4/8] direct-io: add support for fscrypt using blk-crypto

From: Eric Biggers <[email protected]>

Set bio crypt contexts on bios by calling into fscrypt when required,
and explicitly check for DUN continuity when adding pages to the bio.
(While DUN continuity is usually implied by logical block contiguity,
this is not the case when using certain fscrypt IV generation methods
like IV_INO_LBLK_32).

Signed-off-by: Eric Biggers <[email protected]>
Co-developed-by: Satya Tangirala <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
---
fs/direct-io.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index d53fa92a1ab6..f6672c4030e3 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/types.h>
#include <linux/fs.h>
+#include <linux/fscrypt.h>
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/highmem.h>
@@ -392,6 +393,7 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
sector_t first_sector, int nr_vecs)
{
struct bio *bio;
+ struct inode *inode = dio->inode;

/*
* bio_alloc() is guaranteed to return a bio when allowed to sleep and
@@ -399,6 +401,9 @@ dio_bio_alloc(struct dio *dio, struct dio_submit *sdio,
*/
bio = bio_alloc(GFP_KERNEL, nr_vecs);

+ fscrypt_set_bio_crypt_ctx(bio, inode,
+ sdio->cur_page_fs_offset >> inode->i_blkbits,
+ GFP_KERNEL);
bio_set_dev(bio, bdev);
bio->bi_iter.bi_sector = first_sector;
bio_set_op_attrs(bio, dio->op, dio->op_flags);
@@ -763,9 +768,17 @@ static inline int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio,
* current logical offset in the file does not equal what would
* be the next logical offset in the bio, submit the bio we
* have.
+ *
+ * When fscrypt inline encryption is used, data unit number
+ * (DUN) contiguity is also required. Normally that's implied
+ * by logical contiguity. However, certain IV generation
+ * methods (e.g. IV_INO_LBLK_32) don't guarantee it. So, we
+ * must explicitly check fscrypt_mergeable_bio() too.
*/
if (sdio->final_block_in_bio != sdio->cur_page_block ||
- cur_offset != bio_next_offset)
+ cur_offset != bio_next_offset ||
+ !fscrypt_mergeable_bio(sdio->bio, dio->inode,
+ cur_offset >> dio->inode->i_blkbits))
dio_bio_submit(dio, sdio);
}

--
2.30.0.280.ga3ce27912f-goog

2021-01-21 23:30:34

by Satya Tangirala

[permalink] [raw]
Subject: [PATCH v8 2/8] block: blk-crypto: relax alignment requirements for bvecs in bios

blk-crypto only accepted bios whose bvecs' offsets and lengths were aligned
to the crypto data unit size, since blk-crypto-fallback required that to
work correctly.

Now that the blk-crypto-fallback has been updated to work without that
assumption, we relax the alignment requirement - blk-crypto now only needs
the total size of the bio to be aligned to the crypto data unit size.

Co-developed-by: Eric Biggers <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Satya Tangirala <[email protected]>
---
block/blk-crypto.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/block/blk-crypto.c b/block/blk-crypto.c
index 5da43f0973b4..fcee0038f7e0 100644
--- a/block/blk-crypto.c
+++ b/block/blk-crypto.c
@@ -200,22 +200,6 @@ bool bio_crypt_ctx_mergeable(struct bio_crypt_ctx *bc1, unsigned int bc1_bytes,
return !bc1 || bio_crypt_dun_is_contiguous(bc1, bc1_bytes, bc2->bc_dun);
}

-/* Check that all I/O segments are data unit aligned. */
-static bool bio_crypt_check_alignment(struct bio *bio)
-{
- const unsigned int data_unit_size =
- bio->bi_crypt_context->bc_key->crypto_cfg.data_unit_size;
- struct bvec_iter iter;
- struct bio_vec bv;
-
- bio_for_each_segment(bv, bio, iter) {
- if (!IS_ALIGNED(bv.bv_len | bv.bv_offset, data_unit_size))
- return false;
- }
-
- return true;
-}
-
blk_status_t __blk_crypto_init_request(struct request *rq)
{
return blk_ksm_get_slot_for_key(rq->q->ksm, rq->crypt_ctx->bc_key,
@@ -271,7 +255,8 @@ bool __blk_crypto_bio_prep(struct bio **bio_ptr)
goto fail;
}

- if (!bio_crypt_check_alignment(bio)) {
+ if (!IS_ALIGNED(bio->bi_iter.bi_size,
+ bc_key->crypto_cfg.data_unit_size)) {
bio->bi_status = BLK_STS_IOERR;
goto fail;
}
--
2.30.0.280.ga3ce27912f-goog

2021-05-25 18:35:36

by Satya Tangirala

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] add support for direct I/O with fscrypt using blk-crypto

On Tue, May 25, 2021 at 01:57:28PM +0100, Lee Jones wrote:
> On Thu, 21 Jan 2021 at 23:06, Satya Tangirala <[email protected]> wrote:
>
> > This patch series adds support for direct I/O with fscrypt using
> > blk-crypto.
> >
>
> Is there an update on this set please?
>
> I can't seem to find any reviews or follow-up since v8 was posted back in
> January.
>
This patchset relies on the block layer fixes patchset here
https://lore.kernel.org/linux-block/[email protected]/
That said, I haven't been able to actively work on both the patchsets
for a while, but I'll send out updates for both patchsets over the
next week or so.
> --
> Lee Jones [李琼斯]
> Linaro Services Senior Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

2021-05-26 08:13:23

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] add support for direct I/O with fscrypt using blk-crypto

On Tue, 25 May 2021, Satya Tangirala wrote:
65;6200;1c
> On Tue, May 25, 2021 at 01:57:28PM +0100, Lee Jones wrote:
> > On Thu, 21 Jan 2021 at 23:06, Satya Tangirala <[email protected]> wrote:
> >
> > > This patch series adds support for direct I/O with fscrypt using
> > > blk-crypto.
> > >
> >
> > Is there an update on this set please?
> >
> > I can't seem to find any reviews or follow-up since v8 was posted back in
> > January.
> >
> This patchset relies on the block layer fixes patchset here
> https://lore.kernel.org/linux-block/[email protected]/
> That said, I haven't been able to actively work on both the patchsets
> for a while, but I'll send out updates for both patchsets over the
> next week or so.

Thanks Satya, I'd appreciate that.

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

2021-06-09 12:28:26

by Satya Tangirala

[permalink] [raw]
Subject: Re: [PATCH v8 0/8] add support for direct I/O with fscrypt using blk-crypto

On Wed, May 26, 2021 at 09:02:24AM +0100, Lee Jones wrote:
> On Tue, 25 May 2021, Satya Tangirala wrote:
> 65;6200;1c
> > On Tue, May 25, 2021 at 01:57:28PM +0100, Lee Jones wrote:
> > > On Thu, 21 Jan 2021 at 23:06, Satya Tangirala <[email protected]> wrote:
> > >
> > > > This patch series adds support for direct I/O with fscrypt using
> > > > blk-crypto.
> > > >
> > >
> > > Is there an update on this set please?
> > >
> > > I can't seem to find any reviews or follow-up since v8 was posted back in
> > > January.
> > >
> > This patchset relies on the block layer fixes patchset here
> > https://lore.kernel.org/linux-block/[email protected]/
> > That said, I haven't been able to actively work on both the patchsets
> > for a while, but I'll send out updates for both patchsets over the
> > next week or so.
>
> Thanks Satya, I'd appreciate that.
FYI I sent out an updated patch series last week at
https://lore.kernel.org/linux-fscrypt/[email protected]/