diff mbox series

[RFC,2/4] block: add the bi_skip_dm_default_key flag

Message ID 20241018184339.66601-3-ebiggers@kernel.org (mailing list archive)
State New, archived
Headers show
Series dm-default-key: target for filesystem metadata encryption | expand

Commit Message

Eric Biggers Oct. 18, 2024, 6:43 p.m. UTC 
From: Eric Biggers <ebiggers@google.com>

Add a flag bi_skip_dm_default_key to struct bio.  This flag indicates
that dm-default-key should not en/decrypt the bio, due to it targeting
the contents of an encrypted file.

When a bio is cloned, copy the bi_skip_dm_default_key flag.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 block/bio.c                 |  3 +++
 block/blk-crypto-fallback.c |  2 ++
 include/linux/blk-crypto.h  | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/blk_types.h   |  3 +++
 4 files changed, 44 insertions(+)

Comments

Mikulas Patocka Oct. 21, 2024, 11:11 a.m. UTC | #1 
Hi

What about using the REQ_META flag (it is set on metadata bios and cleared 
on data bios), instead of adding a new flag with the same meaning?

Mikulas


On Fri, 18 Oct 2024, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> Add a flag bi_skip_dm_default_key to struct bio.  This flag indicates
> that dm-default-key should not en/decrypt the bio, due to it targeting
> the contents of an encrypted file.
> 
> When a bio is cloned, copy the bi_skip_dm_default_key flag.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  block/bio.c                 |  3 +++
>  block/blk-crypto-fallback.c |  2 ++
>  include/linux/blk-crypto.h  | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/blk_types.h   |  3 +++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/block/bio.c b/block/bio.c
> index ac4d77c889322..5ff0b66e47a42 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -267,10 +267,13 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
>  	bio->bi_iocost_cost = 0;
>  #endif
>  #endif
>  #ifdef CONFIG_BLK_INLINE_ENCRYPTION
>  	bio->bi_crypt_context = NULL;
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +	bio->bi_skip_dm_default_key = false;
> +#endif
>  #endif
>  #ifdef CONFIG_BLK_DEV_INTEGRITY
>  	bio->bi_integrity = NULL;
>  #endif
>  	bio->bi_vcnt = 0;
> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index b1e7415f8439c..dd5f1edcc44b3 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c
> @@ -179,10 +179,12 @@ static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src)
>  	bio_for_each_segment(bv, bio_src, iter)
>  		bio->bi_io_vec[bio->bi_vcnt++] = bv;
>  
>  	bio_clone_blkg_association(bio, bio_src);
>  
> +	bio_clone_skip_dm_default_key(bio, bio_src);
> +
>  	return bio;
>  }
>  
>  static bool
>  blk_crypto_fallback_alloc_cipher_req(struct blk_crypto_keyslot *slot,
> diff --git a/include/linux/blk-crypto.h b/include/linux/blk-crypto.h
> index 5e5822c18ee41..f1f3d546c53e5 100644
> --- a/include/linux/blk-crypto.h
> +++ b/include/linux/blk-crypto.h
> @@ -110,10 +110,13 @@ static inline bool bio_has_crypt_ctx(struct bio *bio)
>  	return false;
>  }
>  
>  #endif /* CONFIG_BLK_INLINE_ENCRYPTION */
>  
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src);
> +
>  int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
>  /**
>   * bio_crypt_clone - clone bio encryption context
>   * @dst: destination bio
>   * @src: source bio
> @@ -125,11 +128,44 @@ int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
>   *	   @gfp_mask doesn't include %__GFP_DIRECT_RECLAIM.
>   */
>  static inline int bio_crypt_clone(struct bio *dst, struct bio *src,
>  				  gfp_t gfp_mask)
>  {
> +	bio_clone_skip_dm_default_key(dst, src);
>  	if (bio_has_crypt_ctx(src))
>  		return __bio_crypt_clone(dst, src, gfp_mask);
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +static inline void bio_set_skip_dm_default_key(struct bio *bio)
> +{
> +	bio->bi_skip_dm_default_key = true;
> +}
> +
> +static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
> +{
> +	return bio->bi_skip_dm_default_key;
> +}
> +
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src)
> +{
> +	dst->bi_skip_dm_default_key = src->bi_skip_dm_default_key;
> +}
> +#else /* CONFIG_DM_DEFAULT_KEY */
> +static inline void bio_set_skip_dm_default_key(struct bio *bio)
> +{
> +}
> +
> +static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
> +{
> +	return false;
> +}
> +
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src)
> +{
> +}
> +#endif /* !CONFIG_DM_DEFAULT_KEY */
> +
>  #endif /* __LINUX_BLK_CRYPTO_H */
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index dce7615c35e7e..2ee6a7e570796 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -247,10 +247,13 @@ struct bio {
>  #endif
>  #endif
>  
>  #ifdef CONFIG_BLK_INLINE_ENCRYPTION
>  	struct bio_crypt_ctx	*bi_crypt_context;
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +	bool			bi_skip_dm_default_key;
> +#endif
>  #endif
>  
>  #if defined(CONFIG_BLK_DEV_INTEGRITY)
>  	struct bio_integrity_payload *bi_integrity; /* data integrity */
>  #endif
> -- 
> 2.47.0
>
Eric Biggers Oct. 21, 2024, 7:02 p.m. UTC | #2 
On Mon, Oct 21, 2024 at 01:11:36PM +0200, Mikulas Patocka wrote:
> Hi
> 
> What about using the REQ_META flag (it is set on metadata bios and cleared 
> on data bios), instead of adding a new flag with the same meaning?
> 
> Mikulas

REQ_META is a hint and is not used for all metadata.

And while metadata is the main point, more precisely the goal is to encrypt
every block that isn't already encrypted.  That means that the contents of files
that are unencrypted at the filesystem layer are encrypted by dm-default-key
too.  So technically it's more than just metadata.

To avoid recurring "oops, we forgot to encrypt this" bugs, the right model is
really an opt-out flag, not opt-in.  And especially not opt-in via something
that is currently just a hint and is used as such.

- Eric
diff mbox series

Patch

diff --git a/block/bio.c b/block/bio.c
index ac4d77c889322..5ff0b66e47a42 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -267,10 +267,13 @@  void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
 	bio->bi_iocost_cost = 0;
 #endif
 #endif
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
 	bio->bi_crypt_context = NULL;
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+	bio->bi_skip_dm_default_key = false;
+#endif
 #endif
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 	bio->bi_integrity = NULL;
 #endif
 	bio->bi_vcnt = 0;
diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index b1e7415f8439c..dd5f1edcc44b3 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -179,10 +179,12 @@  static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src)
 	bio_for_each_segment(bv, bio_src, iter)
 		bio->bi_io_vec[bio->bi_vcnt++] = bv;
 
 	bio_clone_blkg_association(bio, bio_src);
 
+	bio_clone_skip_dm_default_key(bio, bio_src);
+
 	return bio;
 }
 
 static bool
 blk_crypto_fallback_alloc_cipher_req(struct blk_crypto_keyslot *slot,
diff --git a/include/linux/blk-crypto.h b/include/linux/blk-crypto.h
index 5e5822c18ee41..f1f3d546c53e5 100644
--- a/include/linux/blk-crypto.h
+++ b/include/linux/blk-crypto.h
@@ -110,10 +110,13 @@  static inline bool bio_has_crypt_ctx(struct bio *bio)
 	return false;
 }
 
 #endif /* CONFIG_BLK_INLINE_ENCRYPTION */
 
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src);
+
 int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
 /**
  * bio_crypt_clone - clone bio encryption context
  * @dst: destination bio
  * @src: source bio
@@ -125,11 +128,44 @@  int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
  *	   @gfp_mask doesn't include %__GFP_DIRECT_RECLAIM.
  */
 static inline int bio_crypt_clone(struct bio *dst, struct bio *src,
 				  gfp_t gfp_mask)
 {
+	bio_clone_skip_dm_default_key(dst, src);
 	if (bio_has_crypt_ctx(src))
 		return __bio_crypt_clone(dst, src, gfp_mask);
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+static inline void bio_set_skip_dm_default_key(struct bio *bio)
+{
+	bio->bi_skip_dm_default_key = true;
+}
+
+static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
+{
+	return bio->bi_skip_dm_default_key;
+}
+
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src)
+{
+	dst->bi_skip_dm_default_key = src->bi_skip_dm_default_key;
+}
+#else /* CONFIG_DM_DEFAULT_KEY */
+static inline void bio_set_skip_dm_default_key(struct bio *bio)
+{
+}
+
+static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
+{
+	return false;
+}
+
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src)
+{
+}
+#endif /* !CONFIG_DM_DEFAULT_KEY */
+
 #endif /* __LINUX_BLK_CRYPTO_H */
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index dce7615c35e7e..2ee6a7e570796 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -247,10 +247,13 @@  struct bio {
 #endif
 #endif
 
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
 	struct bio_crypt_ctx	*bi_crypt_context;
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+	bool			bi_skip_dm_default_key;
+#endif
 #endif
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 	struct bio_integrity_payload *bi_integrity; /* data integrity */
 #endif