[RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Niels de Vos]

While more filesystems are getting support for fscrypt, it is useful to
be able to disable fscrypt for a selection of filesystems, while
enabling it for others.

The new USE_FS_ENCRYPTION define gets picked up in
include/linux/fscrypt.h. This allows filesystems to choose to use the
empty function definitions, or the functional ones when fscrypt is to be
used with the filesystem.

Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
minimal changes to the filesystems supporting fscrypt. This RFC is
mostly for checking the acceptance of this solution, or if an other
direction is preferred.

---

Niels de Vos (4):
  fscrypt: introduce USE_FS_ENCRYPTION
  fs: make fscrypt support an ext4 config option
  fs: make fscrypt support a f2fs config option
  fs: make fscrypt support a UBIFS config option

 Documentation/filesystems/fscrypt.rst |  2 +-
 fs/crypto/Kconfig                     |  3 +++
 fs/crypto/fscrypt_private.h           |  2 ++
 fs/ext4/Kconfig                       | 13 ++++++++++++-
 fs/ext4/Makefile                      |  2 +-
 fs/ext4/ext4.h                        | 12 ++++++++----
 fs/ext4/inode.c                       |  6 +++---
 fs/ext4/namei.c                       |  6 +++---
 fs/ext4/super.c                       |  6 +++---
 fs/ext4/sysfs.c                       |  8 ++++----
 fs/f2fs/Kconfig                       | 15 +++++++++++++--
 fs/f2fs/data.c                        |  2 +-
 fs/f2fs/dir.c                         |  6 +++---
 fs/f2fs/f2fs.h                        |  8 ++++++--
 fs/f2fs/super.c                       |  6 +++---
 fs/f2fs/sysfs.c                       |  8 ++++----
 fs/ubifs/Kconfig                      | 14 ++++++++++++--
 fs/ubifs/Makefile                     |  2 +-
 fs/ubifs/sb.c                         |  4 ++--
 fs/ubifs/ubifs.h                      |  7 +++++--
 include/linux/fscrypt.h               |  6 +++---
 21 files changed, 93 insertions(+), 45 deletions(-)

-- 
2.37.3

[RFC 1/4] fscrypt: introduce USE_FS_ENCRYPTION

[Niels de Vos]

The new USE_FS_ENCRYPTION define is added so that filesystems can
opt-out of supporting fscrypt, while other filesystems have fscrypt
enabled.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 fs/crypto/fscrypt_private.h | 2 ++
 fs/ext4/ext4.h              | 4 ++++
 fs/f2fs/f2fs.h              | 4 ++++
 fs/ubifs/ubifs.h            | 3 +++
 include/linux/fscrypt.h     | 6 +++---
 5 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index d5f68a0c5d15..f8dc3aab80b3 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -11,6 +11,8 @@
 #ifndef _FSCRYPT_PRIVATE_H
 #define _FSCRYPT_PRIVATE_H
 
+#define USE_FS_ENCRYPTION
+
 #include <linux/fscrypt.h>
 #include <linux/siphash.h>
 #include <crypto/hash.h>
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8d5453852f98..23c2ceaa074d 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -41,7 +41,11 @@
 #include <linux/compat.h>
 #endif
 
+#ifdef CONFIG_FS_ENCRYPTION
+#define USE_FS_ENCRYPTION
+#endif
 #include <linux/fscrypt.h>
+
 #include <linux/fsverity.h>
 
 #include <linux/compiler.h>
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e6355a5683b7..194844029633 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -26,7 +26,11 @@
 #include <linux/part_stat.h>
 #include <crypto/hash.h>
 
+#ifdef CONFIG_FS_ENCRYPTION
+#define USE_FS_ENCRYPTION
+#endif
 #include <linux/fscrypt.h>
+
 #include <linux/fsverity.h>
 
 struct pagevec;
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 478bbbb5382f..3ef0e9ef5015 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -33,6 +33,9 @@
 #include <crypto/hash.h>
 #include <crypto/algapi.h>
 
+#ifdef CONFIG_FS_ENCRYPTION
+#define USE_FS_ENCRYPTION
+#endif
 #include <linux/fscrypt.h>
 
 #include "ubifs-media.h"
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4f5f8a651213..403a686619f8 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -57,7 +57,7 @@ struct fscrypt_name {
 /* Maximum value for the third parameter of fscrypt_operations.set_context(). */
 #define FSCRYPT_SET_CONTEXT_MAX_SIZE	40
 
-#ifdef CONFIG_FS_ENCRYPTION
+#if defined(CONFIG_FS_ENCRYPTION) && defined(USE_FS_ENCRYPTION)
 
 /*
  * If set, the fscrypt bounce page pool won't be allocated (unless another
@@ -379,7 +379,7 @@ static inline void fscrypt_set_ops(struct super_block *sb,
 {
 	sb->s_cop = s_cop;
 }
-#else  /* !CONFIG_FS_ENCRYPTION */
+#else  /* !CONFIG_FS_ENCRYPTION || !USE_FS_ENCRYPTION */
 
 static inline struct fscrypt_info *fscrypt_get_info(const struct inode *inode)
 {
@@ -743,7 +743,7 @@ static inline void fscrypt_set_ops(struct super_block *sb,
 {
 }
 
-#endif	/* !CONFIG_FS_ENCRYPTION */
+#endif	/* !CONFIG_FS_ENCRYPTION || !USE_FS_ENCRYPTION */
 
 /* inline_crypt.c */
 #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
-- 
2.37.3

[RFC 2/4] fs: make fscrypt support an ext4 config option

[Niels de Vos]

Add CONFIG_EXT4_FS_ENCRYPTION as a config option, which depends on the
global CONFIG_FS_ENCRYPTION setting. This makes it possible to opt-out
of fscrypt for ext4 filesystems, while enabling it for others.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 Documentation/filesystems/fscrypt.rst |  2 +-
 fs/crypto/Kconfig                     |  1 +
 fs/ext4/Kconfig                       | 13 ++++++++++++-
 fs/ext4/Makefile                      |  2 +-
 fs/ext4/ext4.h                        | 10 +++++-----
 fs/ext4/inode.c                       |  6 +++---
 fs/ext4/namei.c                       |  6 +++---
 fs/ext4/super.c                       |  6 +++---
 fs/ext4/sysfs.c                       |  8 ++++----
 9 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 5ba5817c17c2..66e3e2afb4a4 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -574,7 +574,7 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors:
 - ``EOPNOTSUPP``: the kernel was not configured with encryption
   support for filesystems, or the filesystem superblock has not
   had encryption enabled on it.  (For example, to use encryption on an
-  ext4 filesystem, CONFIG_FS_ENCRYPTION must be enabled in the
+  ext4 filesystem, CONFIG_EXT4_FS_ENCRYPTION must be enabled in the
   kernel config, and the superblock must have had the "encrypt"
   feature flag enabled using ``tune2fs -O encrypt`` or ``mkfs.ext4 -O
   encrypt``.)
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 2d0c8922f635..7e1267deee51 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -6,6 +6,7 @@ config FS_ENCRYPTION
 	select CRYPTO_SKCIPHER
 	select CRYPTO_LIB_SHA256
 	select KEYS
+	imply EXT4_FS_ENCRYPTION
 	help
 	  Enable encryption of files and directories.  This
 	  feature is similar to ecryptfs, but it is more memory
diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index 86699c8cab28..3108ec1cd046 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -33,7 +33,6 @@ config EXT4_FS
 	select CRYPTO
 	select CRYPTO_CRC32C
 	select FS_IOMAP
-	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
 	help
 	  This is the next generation of the ext3 filesystem.
 
@@ -92,6 +91,18 @@ config EXT4_FS_SECURITY
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
 
+config EXT4_FS_ENCRYPTION
+	bool "Ext4 with support for filesystem encryption"
+	depends on EXT4_FS
+	depends on FS_ENCRYPTION
+	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
+	help
+	  Enable encryption of files and directories. This feature is similar
+	  to ecryptfs, but it is more memory efficient since it avoids caching
+          the encrypted and decrypted pages in the page cache.
+
+	  If unsure, say N.
+
 config EXT4_DEBUG
 	bool "Ext4 debugging support"
 	depends on EXT4_FS
diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 72206a292676..ed4a8232bccf 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -17,4 +17,4 @@ ext4-$(CONFIG_EXT4_FS_SECURITY)		+= xattr_security.o
 ext4-inode-test-objs			+= inode-test.o
 obj-$(CONFIG_EXT4_KUNIT_TESTS)		+= ext4-inode-test.o
 ext4-$(CONFIG_FS_VERITY)		+= verity.o
-ext4-$(CONFIG_FS_ENCRYPTION)		+= crypto.o
+ext4-$(CONFIG_EXT4_FS_ENCRYPTION)	+= crypto.o
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 23c2ceaa074d..a38c50ae742e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -41,7 +41,7 @@
 #include <linux/compat.h>
 #endif
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 #define USE_FS_ENCRYPTION
 #endif
 #include <linux/fscrypt.h>
@@ -2495,7 +2495,7 @@ struct ext4_filename {
 	const struct qstr *usr_fname;
 	struct fscrypt_str disk_name;
 	struct dx_hash_info hinfo;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	struct fscrypt_str crypto_buf;
 #endif
 #if IS_ENABLED(CONFIG_UNICODE)
@@ -2741,7 +2741,7 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
 #endif
 
 /* ext4 encryption related stuff goes here crypto.c */
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 extern const struct fscrypt_operations ext4_cryptops;
 
 int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname,
@@ -2754,7 +2754,7 @@ void ext4_fname_free_filename(struct ext4_filename *fname);
 
 int ext4_ioctl_get_encryption_pwsalt(struct file *filp, void __user *arg);
 
-#else /* !CONFIG_FS_ENCRYPTION */
+#else /* !CONFIG_EXT4_FS_ENCRYPTION */
 static inline int ext4_fname_setup_filename(struct inode *dir,
 					    const struct qstr *iname,
 					    int lookup,
@@ -2792,7 +2792,7 @@ static inline int ext4_ioctl_get_encryption_pwsalt(struct file *filp,
 {
 	return -EOPNOTSUPP;
 }
-#endif /* !CONFIG_FS_ENCRYPTION */
+#endif /* !CONFIG_EXT4_FS_ENCRYPTION */
 
 /* dir.c */
 extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2b5ef1b64249..087dd42ddd42 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1049,7 +1049,7 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
 	return ret;
 }
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
 				  get_block_t *get_block)
 {
@@ -1215,7 +1215,7 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
 	/* In case writeback began while the page was unlocked */
 	wait_for_stable_page(page);
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	if (ext4_should_dioread_nolock(inode))
 		ret = ext4_block_write_begin(page, pos, len,
 					     ext4_get_block_unwritten);
@@ -2999,7 +2999,7 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
 	/* In case writeback began while the page was unlocked */
 	wait_for_stable_page(page);
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	ret = ext4_block_write_begin(page, pos, len,
 				     ext4_da_get_block_prep);
 #else
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index c08c0aba1883..0f61b231ecf6 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -663,7 +663,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 		{
 			if (show_names)
 			{
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 				int len;
 				char *name;
 				struct fscrypt_str fname_crypto_str =
@@ -1475,7 +1475,7 @@ static bool ext4_match(struct inode *parent,
 
 	f.usr_fname = fname->usr_fname;
 	f.disk_name = fname->disk_name;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	f.crypto_buf = fname->crypto_buf;
 #endif
 
@@ -1765,7 +1765,7 @@ static struct buffer_head * ext4_dx_find_entry(struct inode *dir,
 	ext4_lblk_t block;
 	int retval;
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	*res_dir = NULL;
 #endif
 	frame = dx_probe(fname, dir, NULL, frames);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 7cdd2138c897..ef3c7c71ecca 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2000,7 +2000,7 @@ static int ext4_parse_test_dummy_encryption(const struct fs_parameter *param,
 {
 	int err;
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+	if (!IS_ENABLED(CONFIG_EXT4_FS_ENCRYPTION)) {
 		ext4_msg(NULL, KERN_WARNING,
 			 "test_dummy_encryption option not supported");
 		return -EINVAL;
@@ -2122,7 +2122,7 @@ static int ext4_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		ctx_set_mount_flag(ctx, EXT4_MF_FS_ABORTED);
 		return 0;
 	case Opt_inlinecrypt:
-#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+#if defined(CONFIG_EXT4_FS_ENCRYPTION) && defined(CONFIG_FS_ENCRYPTION_INLINE_CRYPT)
 		ctx_set_flags(ctx, SB_INLINECRYPT);
 #else
 		ext4_msg(NULL, KERN_ERR, "inline encryption not supported");
@@ -5241,7 +5241,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	sb->s_op = &ext4_sops;
 	sb->s_export_op = &ext4_export_ops;
 	sb->s_xattr = ext4_xattr_handlers;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	sb->s_cop = &ext4_cryptops;
 #endif
 #ifdef CONFIG_FS_VERITY
diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
index d233c24ea342..148f992dc1a0 100644
--- a/fs/ext4/sysfs.c
+++ b/fs/ext4/sysfs.c
@@ -305,7 +305,7 @@ ATTRIBUTE_GROUPS(ext4);
 EXT4_ATTR_FEATURE(lazy_itable_init);
 EXT4_ATTR_FEATURE(batched_discard);
 EXT4_ATTR_FEATURE(meta_bg_resize);
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 EXT4_ATTR_FEATURE(encryption);
 EXT4_ATTR_FEATURE(test_dummy_encryption_v2);
 #endif
@@ -317,7 +317,7 @@ EXT4_ATTR_FEATURE(verity);
 #endif
 EXT4_ATTR_FEATURE(metadata_csum_seed);
 EXT4_ATTR_FEATURE(fast_commit);
-#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_FS_ENCRYPTION)
+#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_EXT4_FS_ENCRYPTION)
 EXT4_ATTR_FEATURE(encrypted_casefold);
 #endif
 
@@ -325,7 +325,7 @@ static struct attribute *ext4_feat_attrs[] = {
 	ATTR_LIST(lazy_itable_init),
 	ATTR_LIST(batched_discard),
 	ATTR_LIST(meta_bg_resize),
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
 	ATTR_LIST(encryption),
 	ATTR_LIST(test_dummy_encryption_v2),
 #endif
@@ -337,7 +337,7 @@ static struct attribute *ext4_feat_attrs[] = {
 #endif
 	ATTR_LIST(metadata_csum_seed),
 	ATTR_LIST(fast_commit),
-#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_FS_ENCRYPTION)
+#if IS_ENABLED(CONFIG_UNICODE) && defined(CONFIG_EXT4_FS_ENCRYPTION)
 	ATTR_LIST(encrypted_casefold),
 #endif
 	NULL,
-- 
2.37.3

[RFC 3/4] fs: make fscrypt support a f2fs config option

[Niels de Vos]

Add CONFIG_F2FS_FS_ENCRYPTION as a config option, which depends on the
global CONFIG_FS_ENCRYPTION setting. This makes it possible to opt-out
of fscrypt for f2fs, while enabling it for others.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 fs/crypto/Kconfig |  1 +
 fs/f2fs/Kconfig   | 15 +++++++++++++--
 fs/f2fs/data.c    |  2 +-
 fs/f2fs/dir.c     |  6 +++---
 fs/f2fs/f2fs.h    |  6 +++---
 fs/f2fs/super.c   |  6 +++---
 fs/f2fs/sysfs.c   |  8 ++++----
 7 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 7e1267deee51..a809847e820d 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -7,6 +7,7 @@ config FS_ENCRYPTION
 	select CRYPTO_LIB_SHA256
 	select KEYS
 	imply EXT4_FS_ENCRYPTION
+	imply F2FS_FS_ENCRYPTION
 	help
 	  Enable encryption of files and directories.  This
 	  feature is similar to ecryptfs, but it is more memory
diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index 03ef087537c7..801ade82d5c6 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -5,8 +5,6 @@ config F2FS_FS
 	select NLS
 	select CRYPTO
 	select CRYPTO_CRC32
-	select F2FS_FS_XATTR if FS_ENCRYPTION
-	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
 	select FS_IOMAP
 	select LZ4_COMPRESS if F2FS_FS_LZ4
 	select LZ4_DECOMPRESS if F2FS_FS_LZ4
@@ -76,6 +74,19 @@ config F2FS_FS_SECURITY
 
 	  If you are not using a security module, say N.
 
+config F2FS_FS_ENCRYPTION
+	bool "F2FS with support for filesystem encryption"
+	depends on F2FS_FS
+	depends on FS_ENCRYPTION
+	select F2FS_FS_XATTR
+	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
+	help
+	  Enable encryption of files and directories. This feature is similar
+	  to ecryptfs, but it is more memory efficient since it avoids caching
+          the encrypted and decrypted pages in the page cache.
+
+	  If unsure, say N.
+
 config F2FS_CHECK_FS
 	bool "F2FS consistency checking feature"
 	depends on F2FS_FS
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a71e818cd67b..446d2eba964e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -94,7 +94,7 @@ static enum count_type __read_io_type(struct page *page)
 
 /* postprocessing steps for read bios */
 enum bio_post_read_step {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	STEP_DECRYPT	= 1 << 0,
 #else
 	STEP_DECRYPT	= 0,	/* compile out the decryption-related code */
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 21960a899b6a..206580b312fb 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -114,7 +114,7 @@ static int __f2fs_setup_filename(const struct inode *dir,
 
 	fname->usr_fname = crypt_name->usr_fname;
 	fname->disk_name = crypt_name->disk_name;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	fname->crypto_buf = crypt_name->crypto_buf;
 #endif
 	if (crypt_name->is_nokey_name) {
@@ -171,7 +171,7 @@ int f2fs_prepare_lookup(struct inode *dir, struct dentry *dentry,
 
 void f2fs_free_filename(struct f2fs_filename *fname)
 {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	kfree(fname->crypto_buf.name);
 	fname->crypto_buf.name = NULL;
 #endif
@@ -276,7 +276,7 @@ static inline int f2fs_match_name(const struct inode *dir,
 #endif
 	f.usr_fname = fname->usr_fname;
 	f.disk_name = fname->disk_name;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	f.crypto_buf = fname->crypto_buf;
 #endif
 	return fscrypt_match_name(&f, de_name, de_name_len);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 194844029633..fd0da8ce6108 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -26,7 +26,7 @@
 #include <linux/part_stat.h>
 #include <crypto/hash.h>
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 #define USE_FS_ENCRYPTION
 #endif
 #include <linux/fscrypt.h>
@@ -507,7 +507,7 @@ struct f2fs_filename {
 	/* The dirhash of this filename */
 	f2fs_hash_t hash;
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	/*
 	 * For lookups in encrypted directories: either the buffer backing
 	 * disk_name, or a buffer that holds the decoded no-key name.
@@ -4194,7 +4194,7 @@ static inline bool f2fs_encrypted_file(struct inode *inode)
 
 static inline void f2fs_set_encrypted_inode(struct inode *inode)
 {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	file_set_encrypt(inode);
 	f2fs_set_inode_flags(inode);
 #endif
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3834ead04620..224f80bb7eed 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -503,7 +503,7 @@ static int f2fs_set_test_dummy_encryption(struct super_block *sb,
 		&F2FS_OPTION(sbi).dummy_enc_policy;
 	int err;
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION)) {
+	if (!IS_ENABLED(CONFIG_F2FS_FS_ENCRYPTION)) {
 		f2fs_warn(sbi, "test_dummy_encryption option not supported");
 		return -EINVAL;
 	}
@@ -2997,7 +2997,7 @@ static const struct super_operations f2fs_sops = {
 	.remount_fs	= f2fs_remount,
 };
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 static int f2fs_get_context(struct inode *inode, void *ctx, size_t len)
 {
 	return f2fs_getxattr(inode, F2FS_XATTR_INDEX_ENCRYPTION,
@@ -4157,7 +4157,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 #endif
 
 	sb->s_op = &f2fs_sops;
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	sb->s_cop = &f2fs_cryptops;
 #endif
 #ifdef CONFIG_FS_VERITY
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index df27afd71ef4..65e135a84d57 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -848,13 +848,13 @@ F2FS_GENERAL_RO_ATTR(moved_blocks_foreground);
 F2FS_GENERAL_RO_ATTR(avg_vblocks);
 #endif
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 F2FS_FEATURE_RO_ATTR(encryption);
 F2FS_FEATURE_RO_ATTR(test_dummy_encryption_v2);
 #if IS_ENABLED(CONFIG_UNICODE)
 F2FS_FEATURE_RO_ATTR(encrypted_casefold);
 #endif
-#endif /* CONFIG_FS_ENCRYPTION */
+#endif /* CONFIG_F2FS_FS_ENCRYPTION */
 #ifdef CONFIG_BLK_DEV_ZONED
 F2FS_FEATURE_RO_ATTR(block_zoned);
 F2FS_RO_ATTR(F2FS_SBI, f2fs_sb_info, unusable_blocks_per_sec,
@@ -1000,13 +1000,13 @@ static struct attribute *f2fs_attrs[] = {
 ATTRIBUTE_GROUPS(f2fs);
 
 static struct attribute *f2fs_feat_attrs[] = {
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
 	ATTR_LIST(encryption),
 	ATTR_LIST(test_dummy_encryption_v2),
 #if IS_ENABLED(CONFIG_UNICODE)
 	ATTR_LIST(encrypted_casefold),
 #endif
-#endif /* CONFIG_FS_ENCRYPTION */
+#endif /* CONFIG_F2FS_FS_ENCRYPTION */
 #ifdef CONFIG_BLK_DEV_ZONED
 	ATTR_LIST(block_zoned),
 #endif
-- 
2.37.3

[RFC 4/4] fs: make fscrypt support a UBIFS config option

[Niels de Vos]

Add CONFIG_UBIFS_FS_ENCRYPTION as a config option, which depends on the
global CONFIG_FS_ENCRYPTION setting. This makes it possible to opt-out
of fscrypt for UBIFS, while enabling it for others.

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 fs/crypto/Kconfig |  1 +
 fs/ubifs/Kconfig  | 14 ++++++++++++--
 fs/ubifs/Makefile |  2 +-
 fs/ubifs/sb.c     |  4 ++--
 fs/ubifs/ubifs.h  |  6 +++---
 5 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index a809847e820d..2aef21786449 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -8,6 +8,7 @@ config FS_ENCRYPTION
 	select KEYS
 	imply EXT4_FS_ENCRYPTION
 	imply F2FS_FS_ENCRYPTION
+	imply UBIFS_FS_ENCRYPTION
 	help
 	  Enable encryption of files and directories.  This
 	  feature is similar to ecryptfs, but it is more memory
diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index 45d3d207fb99..886056777d68 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -11,8 +11,6 @@ config UBIFS_FS
 	select CRYPTO_DEFLATE if UBIFS_FS_ZLIB
 	select CRYPTO_ZSTD if UBIFS_FS_ZSTD
 	select CRYPTO_HASH_INFO
-	select UBIFS_FS_XATTR if FS_ENCRYPTION
-	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
 	depends on MTD_UBI
 	help
 	  UBIFS is a file system for flash devices which works on top of UBI.
@@ -98,4 +96,16 @@ config UBIFS_FS_AUTHENTICATION
 	  sha256, these are not selected automatically since there are many
 	  different options.
 
+config UBIFS_FS_ENCRYPTION
+	bool "UBIFS with support for filesystem encryption"
+	depends on FS_ENCRYPTION
+	select UBIFS_FS_XATTR
+	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
+	help
+	  Enable encryption of files and directories. This feature is similar
+	  to ecryptfs, but it is more memory efficient since it avoids caching
+          the encrypted and decrypted pages in the page cache.
+
+	  If unsure, say N.
+
 endif # UBIFS_FS
diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
index 314c80b24a76..df49a573f8bd 100644
--- a/fs/ubifs/Makefile
+++ b/fs/ubifs/Makefile
@@ -6,6 +6,6 @@ ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
 ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
 ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o debug.o
 ubifs-y += misc.o sysfs.o
-ubifs-$(CONFIG_FS_ENCRYPTION) += crypto.o
+ubifs-$(CONFIG_UBIFS_FS_ENCRYPTION) += crypto.o
 ubifs-$(CONFIG_UBIFS_FS_XATTR) += xattr.o
 ubifs-$(CONFIG_UBIFS_FS_AUTHENTICATION) += auth.o
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index e7693b94e5b5..1eb2a9be1177 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -746,7 +746,7 @@ int ubifs_read_superblock(struct ubifs_info *c)
 		goto out;
 	}
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION) && c->encrypted) {
+	if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION) && c->encrypted) {
 		ubifs_err(c, "file system contains encrypted files but UBIFS"
 			     " was built without crypto support.");
 		err = -EINVAL;
@@ -932,7 +932,7 @@ int ubifs_enable_encryption(struct ubifs_info *c)
 	int err;
 	struct ubifs_sb_node *sup = c->sup_node;
 
-	if (!IS_ENABLED(CONFIG_FS_ENCRYPTION))
+	if (!IS_ENABLED(CONFIG_UBIFS_FS_ENCRYPTION))
 		return -EOPNOTSUPP;
 
 	if (c->encrypted)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 3ef0e9ef5015..e20f3284f504 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -33,7 +33,7 @@
 #include <crypto/hash.h>
 #include <crypto/algapi.h>
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_UBIFS_FS_ENCRYPTION
 #define USE_FS_ENCRYPTION
 #endif
 #include <linux/fscrypt.h>
@@ -134,7 +134,7 @@
  */
 #define WORST_COMPR_FACTOR 2
 
-#ifdef CONFIG_FS_ENCRYPTION
+#ifdef CONFIG_UBIFS_FS_ENCRYPTION
 #define UBIFS_CIPHER_BLOCK_SIZE FSCRYPT_CONTENTS_ALIGNMENT
 #else
 #define UBIFS_CIPHER_BLOCK_SIZE 0
@@ -2114,7 +2114,7 @@ void ubifs_sysfs_unregister(struct ubifs_info *c);
 #include "misc.h"
 #include "key.h"
 
-#ifndef CONFIG_FS_ENCRYPTION
+#ifndef CONFIG_UBIFS_FS_ENCRYPTION
 static inline int ubifs_encrypt(const struct inode *inode,
 				struct ubifs_data_node *dn,
 				unsigned int in_len, unsigned int *out_len,
-- 
2.37.3

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Theodore Ts'o]

On Thu, Nov 10, 2022 at 03:12:21PM +0100, Niels de Vos wrote:
> While more filesystems are getting support for fscrypt, it is useful to
> be able to disable fscrypt for a selection of filesystems, while
> enabling it for others.

Could you say why you find it useful?  Is it because you are concerned
about the increased binary size of a particular file system if fscrypt
is enabled?  That hasn't been my experience, the hooks to call into
fscrypt are small and don't add too much to any one particular file
system; the bulk of the code is in fs/crypto.

Is it because people are pushing buggy code that doesn't compile if
you enable, say, CONFIG_FS_XXX and CONFIG_FSCRYPT at the same time?

Is it because a particular distribution doesn't want to support
fscrypt with a particular file system?  If so, there have been plenty
of file system features for say, ext4, xfs, and btrfs, which aren't
supported by a distro, but there isn't a CONFIG_FS_XXX_YYY to disable
that feature, nor have any distros requested such a thing --- which is
good because it would be an explosion of new CONFIG parameters.

Or is it something else?

Note that nearly all of the file systems will only enable fscrypt if
some file system feature flag enabls it.  So I'm not sure what's the
motivation behind adding this configuration option.  If memory serves,
early in the fscrypt development we did have per-file system CONFIG's
for fscrypt, but we consciously removed it, just as we no longer have
per-file system CONFIG's to enable or disable Posix ACL's or extended
attributes, in the name of simplifying the kernel config.

Cheers,

						- Ted

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Niels de Vos]

On Thu, Nov 10, 2022 at 10:38:37AM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 03:12:21PM +0100, Niels de Vos wrote:
> > While more filesystems are getting support for fscrypt, it is useful to
> > be able to disable fscrypt for a selection of filesystems, while
> > enabling it for others.
> 
> Could you say why you find it useful?  Is it because you are concerned
> about the increased binary size of a particular file system if fscrypt
> is enabled?  That hasn't been my experience, the hooks to call into
> fscrypt are small and don't add too much to any one particular file
> system; the bulk of the code is in fs/crypto.

No, this isn't really a concern. I don't think the small size difference
is worth the effort.

> Is it because people are pushing buggy code that doesn't compile if
> you enable, say, CONFIG_FS_XXX and CONFIG_FSCRYPT at the same time?

It is a little of this. Not necessarily for the compiling part, more of
a functional thing. And that is what comes next...

> Is it because a particular distribution doesn't want to support
> fscrypt with a particular file system?  If so, there have been plenty
> of file system features for say, ext4, xfs, and btrfs, which aren't
> supported by a distro, but there isn't a CONFIG_FS_XXX_YYY to disable
> that feature, nor have any distros requested such a thing --- which is
> good because it would be an explosion of new CONFIG parameters.

This is mostly why I sent this RFC. We are interested in enabling
fscrypt for CephFS (soonish) as a network filesystem, but not for local
filesystems (we recommend dm-crypt for those). The idea is that
functionality that isn't available, can also not (easily) cause
breakage.

And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...

> Or is it something else?
> 
> Note that nearly all of the file systems will only enable fscrypt if
> some file system feature flag enabls it.  So I'm not sure what's the
> motivation behind adding this configuration option.  If memory serves,
> early in the fscrypt development we did have per-file system CONFIG's
> for fscrypt, but we consciously removed it, just as we no longer have
> per-file system CONFIG's to enable or disable Posix ACL's or extended
> attributes, in the name of simplifying the kernel config.

Thanks for adding some history about this. I understand that extra
options are needed while creating/tuning the filesystems. Preventing
users from setting the right options in a filesystem is not easy, even
if tools from a distribution do not offer setting the options. Disks can
be portable, or network-attached, and have options enabled that an other
distributions kernel does not (want to) support.

Note that even with the additional options, enabling only
CONFIG_FS_ENCRYPTION causes all the filesystems that support fscrypt to
have it enabled. For users there is no change, except that they now have
an option to disable fscrypt support per filesystem.

I hope this explains the intention a bit better. And as there are
existing options for many filesystems to select support for xattrs,
security or ACLs, I hope new options for (de)selecting filesystem
encryption support is not too controversial.

Thanks!
Niels

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Theodore Ts'o]

On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
> CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
> too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...

Actually, I was thinking of getting rid of them, as we've already
gotten rid of EXT4_FS_POSIX_ACL....

> Thanks for adding some history about this. I understand that extra
> options are needed while creating/tuning the filesystems. Preventing
> users from setting the right options in a filesystem is not easy, even
> if tools from a distribution do not offer setting the options. Disks can
> be portable, or network-attached, and have options enabled that an other
> distributions kernel does not (want to) support.

Sure, but as I said, there are **tons** of file system features that
have not and/or still are not supported for distros, but for which we
don't have kernel config knobs.  This includes ext4's bigalloc and
inline data, btrfs's dedup and reflink support, xfs online fsck, etc.,
etc., etc.  Heck, ext4 is only supported up to a certain size by Red
Hat, and we don't have a Kernel config so that the kernel will
absolutely refuse to mount an ext4 file system larger than The
Officially Supported RHEL Capacity Limit for Ext4.  So what makes
fscrypt different from all of these other unsupported file system
features?

There are plenty of times when I've had to explain to customers why,
sure they could build their own kernels for RHEL 4 (back in the day
when I worked for Big Blue and had to talk to lots of enterprise
customers), but if they did, Red Hat support would refuse to give them
the time of day if they called asking for help.  We didn't set up use
digitally signed kernels with trusted boot so that a IBM server would
refuse to boot anything other than An Officially Signed RHEL
Kernel...

What makes fscrypt different that we think we need to enforce this
using technical means, other than a simple, "this feature is not
supported"?

						- Ted

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Theodore Ts'o]

On Thu, Nov 10, 2022 at 01:15:38PM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
> > CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
> > too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...
> 
> Actually, I was thinking of getting rid of them, as we've already
> gotten rid of EXT4_FS_POSIX_ACL....

Sorry, I meant to say that we had gotten rid of EXT4_FS_XATTR.

	       	       	      	      	  - Ted

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Christoph Hellwig]

On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
> CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
> too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...

ext4 is a little weird there as most file systems don't do that.
So I think these should go away for ext4 as well.

> Note that even with the additional options, enabling only
> CONFIG_FS_ENCRYPTION causes all the filesystems that support fscrypt to
> have it enabled. For users there is no change, except that they now have
> an option to disable fscrypt support per filesystem.

But why would you do that anyay?

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Eric Biggers]

On Thu, Nov 10, 2022 at 03:12:21PM +0100, Niels de Vos wrote:
> While more filesystems are getting support for fscrypt, it is useful to
> be able to disable fscrypt for a selection of filesystems, while
> enabling it for others.
> 
> The new USE_FS_ENCRYPTION define gets picked up in
> include/linux/fscrypt.h. This allows filesystems to choose to use the
> empty function definitions, or the functional ones when fscrypt is to be
> used with the filesystem.
> 
> Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
> minimal changes to the filesystems supporting fscrypt. This RFC is
> mostly for checking the acceptance of this solution, or if an other
> direction is preferred.
> 
> ---
> 
> Niels de Vos (4):
>   fscrypt: introduce USE_FS_ENCRYPTION
>   fs: make fscrypt support an ext4 config option
>   fs: make fscrypt support a f2fs config option
>   fs: make fscrypt support a UBIFS config option

So as others have pointed out, it doesn't seem worth the complexity to do this.

For a bit of historical context, before Linux v5.1, we did have per-filesystem
options for this: CONFIG_EXT4_ENCRYPTION, CONFIG_F2FS_FS_ENCRYPTION, and
CONFIG_UBIFS_FS_ENCRYPTION.  If you enabled one of these, it selected
CONFIG_FS_ENCRYPTION to get the code in fs/crypto/.  CONFIG_FS_ENCRYPTION was a
tristate, so the code in fs/crypto/ could be built as a loadable module if it
was only needed by filesystems that were loadable modules themselves.

Having fs/crypto/ possibly be a loadable module was problematic, though, because
it made it impossible to call into fs/crypto/ from built-in code such as
fs/buffer.c, fs/ioctl.c, fs/libfs.c, fs/super.c, fs/iomap/direct-io.c, etc.  So
that's why we made CONFIG_FS_ENCRYPTION into a bool.  At the same time, we
decided to simplify the kconfig options by removing the per-filesystem options
so that it worked like CONFIG_QUOTA, CONFIG_FS_DAX, CONFIG_FS_POSIX_ACL, etc.

I suppose we *could* have *just* changed CONFIG_FS_ENCRYPTION to a bool to solve
the first problem, and kept the per-filesystem options.  I think that wouldn't
have made a lot of sense, though, for the reasons that Ted has already covered.

A further point, beyond what Ted has already covered, is that
non-filesystem-specific code can't honor filesystem-specific options.  So e.g.
if you had a filesystem with encryption disabled by kconfig, that then called
into fs/iomap/direct-io.c to process an I/O request, it could potentially still
call into fs/crypto/ to enable encryption on that I/O request, since
fs/iomap/direct-io.c would think that encryption support is enabled.

Granted, that *should* never actually happen, because this would only make a
difference on encrypted files, and the filesystem shouldn't have allowed an
encrypted file to be opened if it doesn't have encryption support enabled.  But
it does seem a bit odd, given that it would go against the goal of compiling out
all encryption code for a filesystem.

- Eric

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Niels de Vos]

On Thu, Nov 10, 2022 at 01:15:38PM -0500, Theodore Ts'o wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
> > CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
> > too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...
> 
> Actually, I was thinking of getting rid of them, as we've already
> gotten rid of EXT4_FS_POSIX_ACL....
> 
> > Thanks for adding some history about this. I understand that extra
> > options are needed while creating/tuning the filesystems. Preventing
> > users from setting the right options in a filesystem is not easy, even
> > if tools from a distribution do not offer setting the options. Disks can
> > be portable, or network-attached, and have options enabled that an other
> > distributions kernel does not (want to) support.
> 
> Sure, but as I said, there are **tons** of file system features that
> have not and/or still are not supported for distros, but for which we
> don't have kernel config knobs.  This includes ext4's bigalloc and
> inline data, btrfs's dedup and reflink support, xfs online fsck, etc.,
> etc., etc.  Heck, ext4 is only supported up to a certain size by Red
> Hat, and we don't have a Kernel config so that the kernel will
> absolutely refuse to mount an ext4 file system larger than The
> Officially Supported RHEL Capacity Limit for Ext4.  So what makes
> fscrypt different from all of these other unsupported file system
> features?
> 
> There are plenty of times when I've had to explain to customers why,
> sure they could build their own kernels for RHEL 4 (back in the day
> when I worked for Big Blue and had to talk to lots of enterprise
> customers), but if they did, Red Hat support would refuse to give them
> the time of day if they called asking for help.  We didn't set up use
> digitally signed kernels with trusted boot so that a IBM server would
> refuse to boot anything other than An Officially Signed RHEL
> Kernel...
> 
> What makes fscrypt different that we think we need to enforce this
> using technical means, other than a simple, "this feature is not
> supported"?

Thanks again for the added details. What you are explaining makes sense,
and I am not sure if there is an other good reason why splitting out
fscrypt support per filesystem would be required. I'm checking with the
folks that suggested doing this, and see where we go from there.

Niels

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Niels de Vos]

On Sun, Nov 13, 2022 at 10:02:37PM -0800, Christoph Hellwig wrote:
> On Thu, Nov 10, 2022 at 05:47:10PM +0100, Niels de Vos wrote:
> > And, there actually are options like CONFIG_EXT4_FS_POSIX_ACL and
> > CONFIG_EXT4_FS_SECURITY. Because these exist already, I did not expect
> > too much concerns with proposing a CONFIG_EXT4_FS_ENCRYPTION...
> 
> ext4 is a little weird there as most file systems don't do that.
> So I think these should go away for ext4 as well.

Yeah, I understand that there is a preference for reducing the number of
Kconfig options for filesystems. That indeed would make it a little
easier for users, so I am supportive of that as well.

> > Note that even with the additional options, enabling only
> > CONFIG_FS_ENCRYPTION causes all the filesystems that support fscrypt to
> > have it enabled. For users there is no change, except that they now have
> > an option to disable fscrypt support per filesystem.
> 
> But why would you do that anyay?

An other mail in this thread contains a description about that. It is
more about being able to provide a kernel build that is fully tested,
and enabling more options (or being unable to disable features)
increases the testing efforts that are needed.

However, as Ted pointed out, there are other features that can not be
disabled or limited per filesystem, so there will always be a gap in
what can practically be tested.

Thanks,
Niels

Re: [RFC 0/4] fs: provide per-filesystem options to disable fscrypt

[Niels de Vos]

On Tue, Nov 15, 2022 at 06:10:59PM -0800, Eric Biggers wrote:
> On Thu, Nov 10, 2022 at 03:12:21PM +0100, Niels de Vos wrote:
> > While more filesystems are getting support for fscrypt, it is useful to
> > be able to disable fscrypt for a selection of filesystems, while
> > enabling it for others.
> > 
> > The new USE_FS_ENCRYPTION define gets picked up in
> > include/linux/fscrypt.h. This allows filesystems to choose to use the
> > empty function definitions, or the functional ones when fscrypt is to be
> > used with the filesystem.
> > 
> > Using USE_FS_ENCRYPTION is a relatively clean approach, and requires
> > minimal changes to the filesystems supporting fscrypt. This RFC is
> > mostly for checking the acceptance of this solution, or if an other
> > direction is preferred.
> > 
> > ---
> > 
> > Niels de Vos (4):
> >   fscrypt: introduce USE_FS_ENCRYPTION
> >   fs: make fscrypt support an ext4 config option
> >   fs: make fscrypt support a f2fs config option
> >   fs: make fscrypt support a UBIFS config option
> 
> So as others have pointed out, it doesn't seem worth the complexity to do this.
> 
> For a bit of historical context, before Linux v5.1, we did have per-filesystem
> options for this: CONFIG_EXT4_ENCRYPTION, CONFIG_F2FS_FS_ENCRYPTION, and
> CONFIG_UBIFS_FS_ENCRYPTION.  If you enabled one of these, it selected
> CONFIG_FS_ENCRYPTION to get the code in fs/crypto/.  CONFIG_FS_ENCRYPTION was a
> tristate, so the code in fs/crypto/ could be built as a loadable module if it
> was only needed by filesystems that were loadable modules themselves.
> 
> Having fs/crypto/ possibly be a loadable module was problematic, though, because
> it made it impossible to call into fs/crypto/ from built-in code such as
> fs/buffer.c, fs/ioctl.c, fs/libfs.c, fs/super.c, fs/iomap/direct-io.c, etc.  So
> that's why we made CONFIG_FS_ENCRYPTION into a bool.  At the same time, we
> decided to simplify the kconfig options by removing the per-filesystem options
> so that it worked like CONFIG_QUOTA, CONFIG_FS_DAX, CONFIG_FS_POSIX_ACL, etc.
> 
> I suppose we *could* have *just* changed CONFIG_FS_ENCRYPTION to a bool to solve
> the first problem, and kept the per-filesystem options.  I think that wouldn't
> have made a lot of sense, though, for the reasons that Ted has already covered.

Yes, it seems that there is a move to reduce the Kconfig options and
(re)adding per-filesystem encryption support would be counterproductive.

> A further point, beyond what Ted has already covered, is that
> non-filesystem-specific code can't honor filesystem-specific options.  So e.g.
> if you had a filesystem with encryption disabled by kconfig, that then called
> into fs/iomap/direct-io.c to process an I/O request, it could potentially still
> call into fs/crypto/ to enable encryption on that I/O request, since
> fs/iomap/direct-io.c would think that encryption support is enabled.
> 
> Granted, that *should* never actually happen, because this would only make a
> difference on encrypted files, and the filesystem shouldn't have allowed an
> encrypted file to be opened if it doesn't have encryption support enabled.  But
> it does seem a bit odd, given that it would go against the goal of compiling out
> all encryption code for a filesystem.

Ah, yes, indeed! The boundaries between the options would be less clear,
and potential changes to shared functions under fs/ could have incorrect
assumptions about CONFIG_FS_ENCRYPTION. Even if this is not the case
now, optimizations/enhancements in the future might be more complicated
because of this.

Thanks for the additional details! Have a good weekend,
Niels