ecryptfs-devel team mailing list archive

[Dustin Kirkland <kirkland@xxxxxxxxxxxxx>]

On Tue, Sep 22, 2009 at 2:49 PM, Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx> wrote:
> When calling vfs_unlink() on the lower dentry, d_delete() turns the
> dentry into a negative dentry when the d_count is 1.  This eventually
> caused a NULL pointer deref when a read() or write() was done and the
> negative dentry's d_inode was dereferenced in
> ecryptfs_read_update_atime() or ecryptfs_getxattr().
>
> Placing mutt's tmpdir in an eCryptfs mount is what initially triggered
> the oops and I was able to reproduce it with the following sequence:
>
> open("/tmp/upper/foo", O_RDWR|O_CREAT|O_EXCL|O_NOFOLLOW, 0600) = 3
> link("/tmp/upper/foo", "/tmp/upper/bar") = 0
> unlink("/tmp/upper/foo")                = 0
> open("/tmp/upper/bar", O_RDWR|O_CREAT|O_NOFOLLOW, 0600) = 4
> unlink("/tmp/upper/bar")                = 0
> write(4, "eCryptfs test\n"..., 14 <unfinished ...>
> +++ killed by SIGKILL +++
>
> https://bugs.launchpad.net/ecryptfs/+bug/387073
>
> Reported-by: Loïc Minier <loic.minier@xxxxxxxxxxxxx>
> Cc: Serge Hallyn <serue@xxxxxxxxxx>
> Cc: Dave Kleikamp <shaggy@xxxxxxxxxxxxxxxxxx>
> Cc: ecryptfs-devel@xxxxxxxxxxxxxxxxxxx
> Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx>
> ---
>  fs/ecryptfs/inode.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 2f0945d..056fed6 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -476,6 +476,7 @@ static int ecryptfs_unlink(struct inode *dir, struct dentry *dentry)
>        struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
>        struct dentry *lower_dir_dentry;
>
> +       dget(lower_dentry);
>        lower_dir_dentry = lock_parent(lower_dentry);
>        rc = vfs_unlink(lower_dir_inode, lower_dentry);
>        if (rc) {
> @@ -489,6 +490,7 @@ static int ecryptfs_unlink(struct inode *dir, struct dentry *dentry)
>        d_drop(dentry);
>  out_unlock:
>        unlock_dir(lower_dir_dentry);
> +       dput(lower_dentry);
>        return rc;
>  }

Tyler-

Patch looks good.  Are you going to send this to Linus for the stable tree?

:-Dustin