ecryptfs-devel team mailing list archive

[Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx>]

On 09/22/2009 05:12 PM, Dustin Kirkland wrote:
> 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

I'm going to send Linus a pull request for this patch and several others
in a a day or two.