Re: ecryptfs

[Michael Halcrow <mhalcrow@us.ibm.com>]

***DO NOT ACCEPT THIS PATCH***

On Mon, Aug 07, 2006 at 11:19:39AM -0500, Michael Halcrow wrote:
> I have a couple more bogus patches I would like feedback on; they're
> bogus at this stage because I am struggling to get them to work
> right. I'll send them as a follow-on to this message.

***DO NOT ACCEPT THIS PATCH***

This code is for commentary only at this stage. Note that it applies
on top of a patch that associates the vfsmount with the dentry rather
than the superblock.

Here is where I am thinking about going with crossing lower mount
points. This patch makes sure that there is a 1-to-1 mapping in inode
numbers between the stacked inodes and the lower inodes. It maintains
the association by modifying the struct inode to include a back
pointer from the lower inode to the stacked inode. Note that Unionfs
has a persistent inode approach that is much more heavy-weight than
this approach, but it does not need to modify anything in the VFS.

I would like the CONFIG_STACK_FS to govern VFS modifications that make
stackable filesystems more functional. My approach is to allow the
kernel to be built without any such modifications, but certain
features will be disabled/degraded in the stacked filesystem.

Unfortunately, with this patch, things blow up on umount; I'll have to
investigate what's going on.

diff --git a/fs/Kconfig b/fs/Kconfig
index afacf48..30c78b9 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -443,6 +443,14 @@ config QUOTA
 	  with the quota tools. Probably the quota support is only useful for
 	  multi user systems. If unsure, say N.
 
+config STACK_FS
+	bool "Stacked filesystem support"
+	default y
+	help
+	  Select Y to enable all features in stacked filesystems (such
+	  as eCryptfs and Unionfs).  Slightly increases the size of
+	  inodes in memory.  If unsure, say Y.
+
 config QFMT_V1
 	tristate "Old quota format support"
 	depends on QUOTA
@@ -984,6 +992,9 @@ config ECRYPT_FS
 	  Encrypted filesystem that operates on the VFS layer.  See
 	  Documentation/ecryptfs.txt to learn more about eCryptfs.
 
+	  It is strongly recommended that you also enable ``Stacked
+	  filesystem support'' (CONFIG_STACK_FS) if you build eCryptfs.
+
 	  To compile this file system support as a module, choose M here: the
 	  module will be called ecryptfs.
 
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 349ce2a..3e8f377 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -376,6 +376,51 @@ ecryptfs_set_dentry_lower_mnt(struct den
 		lower_mnt;
 }
 
+#ifdef CONFIG_STACK_FS
+static inline int
+ecryptfs_get_inode(struct inode **inode, struct inode *lower_inode,
+		   struct super_block *sb)
+{
+	*inode = lower_inode->i_stacked_inode;
+	if (!*inode) {
+		*inode = iget(sb, iunique(sb, 0));
+		if (!*inode)
+			return -EACCES;
+		lower_inode->i_stacked_inode = *inode;
+	}
+	return 0;
+}
+#else
+static inline int
+ecryptfs_get_inode(struct inode **inode, struct inode *lower_inode,
+		   struct super_block *sb)
+{
+	/* Without CONFIG_STACK_FS enabled, we have no way to safely
+	 * cross mount points in the lower filesystem. */
+	if (lower_inode->i_sb != ecryptfs_superblock_to_lower(sb)) {
+		printk(KERN_ERR "Cannot cross mount point in lower filesystem; "
+		       "recompile your kernel with CONFIG_STACK_FS to enable "
+		       "this feature.\n");
+		return -EXDEV;
+	}
+	*inode = iget(sb, lower_inode->i_ino);
+	if (!*inode)
+		return -EACCES;
+	return 0;
+}
+#endif /* CONFIG_STACK_FS */
+
+#ifdef CONFIG_STACK_FS
+static inline void ecryptfs_clear_inode_stacking(struct inode *inode)
+{
+	struct inode *lower_inode = ecryptfs_inode_to_lower(inode);
+
+	lower_inode->i_stacked_inode = NULL;
+}
+#else
+static inline void ecryptfs_clear_inode_stacking(struct inode *inode) { }
+#endif /* CONFIG_STACK_FS */
+
 #define ecryptfs_printk(type, fmt, arg...) \
         __ecryptfs_printk(type "%s: " fmt, __FUNCTION__, ## arg);
 void __ecryptfs_printk(const char *fmt, ...);
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index d2e1cb0..119c0a5 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -77,22 +77,16 @@ int ecryptfs_interpose(struct dentry *lo
 		       struct super_block *sb, int flag)
 {
 	struct inode *lower_inode;
-	int rc = 0;
 	struct inode *inode;
+	int rc = 0;
 
 	lower_inode = lower_dentry->d_inode;
-	if (lower_inode->i_sb != ecryptfs_superblock_to_lower(sb)) {
-		rc = -EXDEV;
-		goto out;
-	}
-	inode = iget(sb, lower_inode->i_ino);
-	if (!inode) {
-		rc = -EACCES;
+	rc = ecryptfs_get_inode(&inode, lower_inode, sb);
+	if (rc) {
+		printk(KERN_ERR "Error getting inode\n");
 		goto out;
 	}
-	/* This check is required here because if we failed to allocated the
-	 * required space for an inode_info_cache struct, then the only way
-	 * we know we failed, is by the pointer being NULL */
+	/* Did we succeed in allocating ecryptfs_inode_info? */
 	if (!ecryptfs_inode_to_private(inode)) {
 		ecryptfs_printk(KERN_ERR, "Out of memory. Failure to "
 				"allocate memory in ecryptfs_read_inode.\n");
diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c
index 120f060..d96aa65 100644
--- a/fs/ecryptfs/super.c
+++ b/fs/ecryptfs/super.c
@@ -138,6 +138,7 @@ static int ecryptfs_statfs(struct dentry
  */
 static void ecryptfs_clear_inode(struct inode *inode)
 {
+	ecryptfs_clear_inode_stacking(inode);
 	iput(ecryptfs_inode_to_lower(inode));
 }
 
diff --git a/fs/inode.c b/fs/inode.c
index 89ce5ba..3d3efc2 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -163,7 +163,10 @@ #endif
 				bdi = sb->s_bdev->bd_inode->i_mapping->backing_dev_info;
 			mapping->backing_dev_info = bdi;
 		}
-		inode->i_private = 0;
+		inode->i_private = NULL;
+#ifdef CONFIG_STACK_FS
+		inode->i_stacked_inode = NULL;
+#endif
 		inode->i_mapping = mapping;
 	}
 	return inode;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6cd7004..306a028 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -578,6 +578,9 @@ #endif
 	atomic_t		i_writecount;
 	void			*i_security;
 	void			*i_private; /* fs or device private pointer */
+#ifdef CONFIG_STACK_FS
+	struct inode		*i_stacked_inode; /* stackable fs support */
+#endif
 #ifdef __NEED_I_SIZE_ORDERED
 	seqcount_t		i_size_seqcount;
 #endif