public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michael Halcrow <mhalcrow@us.ibm.com>
To: jsipek@fsl.cs.sunysb.edu, Andrew Morton <akpm@osdl.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"TREVOR S. HIGHLAND" <tshighla@us.ibm.com>
Subject: [PATCH] fsstack: Remove inode copy
Date: Fri, 15 Dec 2006 10:24:28 -0600	[thread overview]
Message-ID: <20061215162428.GA3570@us.ibm.com> (raw)

Trevor found a file size problem in eCryptfs in recent kernels, and he
tracked it down to an fsstack change.

This was the eCryptfs copy_attr_all:

> -void ecryptfs_copy_attr_all(struct inode *dest, const struct inode *src)
> -{
> -       dest->i_mode = src->i_mode;
> -       dest->i_nlink = src->i_nlink;
> -       dest->i_uid = src->i_uid;
> -       dest->i_gid = src->i_gid;
> -       dest->i_rdev = src->i_rdev;
> -       dest->i_atime = src->i_atime;
> -       dest->i_mtime = src->i_mtime;
> -       dest->i_ctime = src->i_ctime;
> -       dest->i_blkbits = src->i_blkbits;
> -       dest->i_flags = src->i_flags;
> -}

This is the fsstack copy_attr_all:

> +void fsstack_copy_attr_all(struct inode *dest, const struct inode *src,
> +                               int (*get_nlinks)(struct inode *))
> +{
> +       if (!get_nlinks)
> +               dest->i_nlink = src->i_nlink;
> +       else
> +               dest->i_nlink = (*get_nlinks)(dest);
> +
> +       dest->i_mode = src->i_mode;
> +       dest->i_uid = src->i_uid;
> +       dest->i_gid = src->i_gid;
> +       dest->i_rdev = src->i_rdev;
> +       dest->i_atime = src->i_atime;
> +       dest->i_mtime = src->i_mtime;
> +       dest->i_ctime = src->i_ctime;
> +       dest->i_blkbits = src->i_blkbits;
> +       dest->i_flags = src->i_flags;
> +
> +       fsstack_copy_inode_size(dest, src);
> +}

The addition of copy_inode_size breaks eCryptfs, since eCryptfs needs
to interpolate the file sizes (eCryptfs has extra space in the lower
file for the header). The setting of the upper inode size occurs
elsewhere in eCryptfs, and the new copy_attr_all now undoes what
eCryptfs was doing right beforehand.

I see three ways of going forward from here. (1) Something like this
patch needs to go in (assuming it jives with Unionfs), (2) we need to
make a change to the fsstack API for more fine-grained control over
copying attributes (e.g., by also including a callback function for
calculating the right file size, which will require some more work on
both eCryptfs and Unionfs), or (3) the fsstack patch on eCryptfs
(commit 0cc72dc7f050188d8d7344b1dd688cbc68d3cd30 made on Fri Dec 8
02:36:31 2006 -0800) needs to be yanked in 2.6.20.

I think the simplest solution, from eCryptfs' perspective, is to just
remove the inode size copy. Jeff, please let me know if this approach
will work for you, or let me know if you have another idea.

Thanks,
Mike

---

Remove inode size copy in general fsstack attr copy code. Stacked
filesystems may need to interpolate the inode size, since the file
size in the lower file may be different than the file size in the
stacked layer.

Signed-off-by: Michael Halcrow <mhalcrow@us.ibm.com>

---

 fs/stack.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

97582cdc0c45a37fb75488a96b02995a7b28364d
diff --git a/fs/stack.c b/fs/stack.c
index 5ddbc34..8ffb880 100644
--- a/fs/stack.c
+++ b/fs/stack.c
@@ -34,7 +34,5 @@ void fsstack_copy_attr_all(struct inode 
 	dest->i_ctime = src->i_ctime;
 	dest->i_blkbits = src->i_blkbits;
 	dest->i_flags = src->i_flags;
-
-	fsstack_copy_inode_size(dest, src);
 }
 EXPORT_SYMBOL_GPL(fsstack_copy_attr_all);
-- 
1.3.3


             reply	other threads:[~2006-12-15 16:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-15 16:24 Michael Halcrow [this message]
2006-12-15 19:53 ` [PATCH] fsstack: Remove inode copy Josef Sipek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061215162428.GA3570@us.ibm.com \
    --to=mhalcrow@us.ibm.com \
    --cc=akpm@osdl.org \
    --cc=jsipek@fsl.cs.sunysb.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tshighla@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox