Re: [PATCH] ovl: fix dir cache leak for ovl_cache_get

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 5, 2017 at 9:34 AM, Rock Lee <rockdotlee@xxxxxxxxx> wrote:
> Hi, Amir:
>
>> This is not a leak. dir cache is refcounted and freed on
>> ovl_dir_{release|reset}() -> ovl_cache_put()
>
> Here is the snippet:
>
> if (cache && ovl_dentry_version_get(dentry) == cache->version) {
> cache->refcount++;
> return cache;
> }
>
> ovl_set_dir_cache(dentry, NULL);
>
> I am a little confused here. From the logic of the code, there must
> have a circumstance --- cache is not NULL and
> ovl_dentry_version_get(dentry) != cache->version. If
> ovl_set_dir_cache(dentry, NULL), we can't find the old dir cache  form
> dentry and struct file(later, file->private_data->cache will point at
> the same dir cache with dentry). How could we free it in
> ovl_dir_{release|reset} ?
>

It has a reference from another struct file, not this one
and cache will be freed when the last file that references it
is closed:

static int ovl_dir_release(struct inode *inode, struct file *file)
{
        struct ovl_dir_file *od = file->private_data;

        if (od->cache) {
                inode_lock(inode);
                ovl_cache_put(od, file->f_path.dentry);
                inode_unlock(inode);
        }

> BTW, could you please an example when ovl_dentry_version_get(dentry)
> != cache->version in ovl_cache_get ?
>

In theory (didn't try this code):

d1 = opendir("foo");
readdir(d1); /* populate d1's od->cache and read first entry */
pos = telldir(d1);
creat("foo/bar", 0600); /* ovl_dentry_version_inc() */
d2 = opendir("foo");
seekdir(d2, pos);
readdir(d2); /* populate d2's od->cache and read second entry */

d2's od->cache is NULL, ctx->pos = 1, and ovl_get_cache() will
find that ovl_dir_cache() is out dated and will create a new cache,
which should contain the new file "bar".

Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux