On Wed, Sep 20, 2017 at 03:45:40PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> This series reduces code duplication among ext4, f2fs, and ubifs by
> introducing a S_ENCRYPTED inode flag (so we don't have to call back into
> the filesystem to test the filesystem-specific inode flag), then
> introducing new helper functions that are called at the beginning of the
> open, link, rename, lookup, and setattr operations.
>
> In the future we maybe should even call these new helpers from the VFS
> so that each individual filesystem doesn't have to do it. But that's
> not possible currently because fs/crypto/ can be built as a module.
>
> Making changes like this is a bit challenging due to interdependencies
> between fscrypt and the individual filesystems, all of which have
> different maintainers. For now my intent is that patches 1-10 be taken
> through the fscrypt tree --- though it's not perfect since patches 1-4
> do make some changes to each filesystem, as everyone must set
> S_ENCRYPTED before we can use it everywhere in the shared code. But
> afterwards, patches 11-25 can be picked up by the individual filesystems
> to switch to the new helpers.
This all looks much nicer. Having just been looking at this stuff,
it makes the code much simpler to understand. So:
Acked-by: Dave Chinner <dchinner@xxxxxxxxxx>
While I'm here, the fscrypt header file includes are clunky and
nasty. I worte a quick patch a couple of days ago to clean it up.
See below....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
fscrypto: clean up include file mess
From: Dave Chinner <dchinner@xxxxxxxxxx>
Filesystems have to include different header files based on whether
they are compiled with encryption support or not. That's nasty and
messy.
Instead, rationalise the headers so we have a single include
fscrypt.h and let it decide what internal implementation to include
based on the __FS_HAS_ENCRYPTION define. Filesystems set
__FS_HAS_ENCRYPTION before including linux/fscrypt.h if they are
built with encryption support.
Add guards to prevent fscrypt_supp.h and fscrypt_notsupp.h from
being directly included by filesystems.
Signed-Off-By: Dave Chinner <dchinner@xxxxxxxxxx>
---
fs/crypto/fscrypt_private.h | 3 +-
fs/ext4/ext4.h | 11 +++----
fs/f2fs/f2fs.h | 8 +++---
fs/ubifs/ubifs.h | 9 +++---
include/linux/{fscrypt_common.h => fscrypt.h} | 41 ++++++++++++++++++---------
include/linux/fscrypt_notsupp.h | 7 +++--
include/linux/fscrypt_supp.h | 7 +++--
7 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a1d5021c31ef..a180981ee6d7 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -11,7 +11,8 @@
#ifndef _FSCRYPT_PRIVATE_H
#define _FSCRYPT_PRIVATE_H
-#include <linux/fscrypt_supp.h>
+#define __FS_HAS_ENCRYPTION 1
+#include <linux/fscrypt.h>
#include <crypto/hash.h>
/* Encryption parameters */
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index e2abe01c8c6b..900ac79879b3 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -33,17 +33,18 @@
#include <linux/percpu_counter.h>
#include <linux/ratelimit.h>
#include <crypto/hash.h>
-#ifdef CONFIG_EXT4_FS_ENCRYPTION
-#include <linux/fscrypt_supp.h>
-#else
-#include <linux/fscrypt_notsupp.h>
-#endif
+
#include <linux/falloc.h>
#include <linux/percpu-rwsem.h>
#ifdef __KERNEL__
#include <linux/compat.h>
#endif
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
+#define __FS_HAS_ENCRYPTION 1
+#endif
+#include <linux/fscrypt.h>
+
/*
* The fourth extended filesystem constants/structures
*/
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9a7c90386947..66502c5f7d67 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -23,12 +23,12 @@
#include <linux/bio.h>
#include <linux/blkdev.h>
#include <linux/quotaops.h>
+#include <crypto/hash.h>
+
#ifdef CONFIG_F2FS_FS_ENCRYPTION
-#include <linux/fscrypt_supp.h>
-#else
-#include <linux/fscrypt_notsupp.h>
+#define __FS_HAS_ENCRYPTION 1
#endif
-#include <crypto/hash.h>
+#include <linux/fscrypt.h>
#ifdef CONFIG_F2FS_CHECK_FS
#define f2fs_bug_on(sbi, condition) BUG_ON(condition)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index cd43651f1731..e5b6c8f02133 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -38,12 +38,13 @@
#include <linux/backing-dev.h>
#include <linux/security.h>
#include <linux/xattr.h>
+#include <linux/random.h>
+
#ifdef CONFIG_UBIFS_FS_ENCRYPTION
-#include <linux/fscrypt_supp.h>
-#else
-#include <linux/fscrypt_notsupp.h>
+#define __FS_HAS_ENCRYPTION 1
#endif
-#include <linux/random.h>
+#include <linux/fscrypt.h>
+
#include "ubifs-media.h"
/* Version of this UBIFS implementation */
diff --git a/include/linux/fscrypt_common.h b/include/linux/fscrypt.h
similarity index 79%
rename from include/linux/fscrypt_common.h
rename to include/linux/fscrypt.h
index 97f738628b36..4db0a7ec26d9 100644
--- a/include/linux/fscrypt_common.h
+++ b/include/linux/fscrypt.h
@@ -1,14 +1,17 @@
/*
- * fscrypt_common.h: common declarations for per-file encryption
+ * fscrypt.h: declarations for per-file encryption
+ *
+ * Filesystems that implement per-file encryption include this header
+ * file with the __FS_HAS_ENCRYPTION set according to whether that filesystem
+ * is being built with encryption support or not.
*
* Copyright (C) 2015, Google, Inc.
*
* Written by Michael Halcrow, 2015.
* Modified by Jaegeuk Kim, 2015.
*/
-
-#ifndef _LINUX_FSCRYPT_COMMON_H
-#define _LINUX_FSCRYPT_COMMON_H
+#ifndef _LINUX_FSCRYPT_H
+#define _LINUX_FSCRYPT_H
#include <linux/key.h>
#include <linux/fs.h>
@@ -119,23 +122,35 @@ static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
return false;
}
+#ifdef __FS_HAS_ENCRYPTION
+
static inline struct page *fscrypt_control_page(struct page *page)
{
-#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
return ((struct fscrypt_ctx *)page_private(page))->w.control_page;
-#else
+}
+
+static inline bool fscrypt_has_encryption_key(const struct inode *inode)
+{
+ return (inode->i_crypt_info != NULL);
+}
+
+#include <linux/fscrypt_supp.h>
+
+#else /* !__FS_HAS_ENCRYPTION */
+
+static inline struct page *fscrypt_control_page(struct page *page)
+{
WARN_ON_ONCE(1);
return ERR_PTR(-EINVAL);
-#endif
}
-static inline int fscrypt_has_encryption_key(const struct inode *inode)
+static inline bool fscrypt_has_encryption_key(const struct inode *inode)
{
-#if IS_ENABLED(CONFIG_FS_ENCRYPTION)
- return (inode->i_crypt_info != NULL);
-#else
return 0;
-#endif
}
-#endif /* _LINUX_FSCRYPT_COMMON_H */
+#include <linux/fscrypt_notsupp.h>
+#endif /* __FS_HAS_ENCRYPTION */
+
+
+#endif /* _LINUX_FSCRYPT_H */
diff --git a/include/linux/fscrypt_notsupp.h b/include/linux/fscrypt_notsupp.h
index ec406aed2f2f..2d0b6960831e 100644
--- a/include/linux/fscrypt_notsupp.h
+++ b/include/linux/fscrypt_notsupp.h
@@ -3,13 +3,16 @@
*
* This stubs out the fscrypt functions for filesystems configured without
* encryption support.
+ *
+ * Do not include this file directly. Use fscrypt.h instead!
*/
+#ifndef _LINUX_FSCRYPT_H
+#error "Incorrect include of linux/fscrypt_notsupp.h!"
+#endif
#ifndef _LINUX_FSCRYPT_NOTSUPP_H
#define _LINUX_FSCRYPT_NOTSUPP_H
-#include <linux/fscrypt_common.h>
-
/* crypto.c */
static inline struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode,
gfp_t gfp_flags)
diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h
index 32e2fcf13b01..5a90e5ef4687 100644
--- a/include/linux/fscrypt_supp.h
+++ b/include/linux/fscrypt_supp.h
@@ -1,14 +1,15 @@
/*
* fscrypt_supp.h
*
- * This is included by filesystems configured with encryption support.
+ * Do not include this file directly. Use fscrypt.h instead!
*/
+#ifndef _LINUX_FSCRYPT_H
+#error "Incorrect include of linux/fscrypt_supp.h!"
+#endif
#ifndef _LINUX_FSCRYPT_SUPP_H
#define _LINUX_FSCRYPT_SUPP_H
-#include <linux/fscrypt_common.h>
-
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
extern struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *, gfp_t);
