Re: [PATCH 06/20] sysfs: Use dentry_ops instead of directly playing with the dcache

Tejun Heo
2009-05-21T06:41:25Z

Hello,

Eric W. Biederman wrote:
> Calling d_drop unconditionally when a sysfs_dirent is deleted has
> the potential to leak mounts, so instead implement dentry delete
> and revalidate operations that cause sysfs dentries to be removed
> at the appropriate time.
>
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>

Great, thanks for doing this. It's much better than the fragile vfs
tinkering sysfs has been doing.

> +static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
> +{
> + struct sysfs_dirent *sd = dentry->d_fsdata;
> + int is_dir;
> +
> + mutex_lock(&sysfs_mutex);
> +
> + /* The sysfs dirent has been deleted */
> + if (sd->s_flags & SYSFS_FLAG_REMOVED)
> + goto out_bad;
> +
> + mutex_unlock(&sysfs_mutex);
> +out_valid:
> + return 1;
> +out_bad:
> + /* Remove the dentry from the dcache hashes.
> + * If this is a deleted dentry we use d_drop instead of d_delete
> + * so sysfs doesn't need to cope with negative dentries.
> + */
> + is_dir = (sysfs_type(sd) == SYSFS_DIR);
> + mutex_unlock(&sysfs_mutex);
> + if (is_dir) {
> + /* If we have submounts we must allow the vfs caches
> + * to lie about the state of the filesystem to prevent
> + * leaks and other nasty things.
> + */
> + if (have_submounts(dentry))
> + goto out_valid;
> + shrink_dcache_parent(dentry);
> + }
> + d_drop(dentry);
> + return 0;
> +}

Ummm... what happens if sysfs recreates those identical nodes again
while the old dentries are lingering? The dead ones will linger till
the submounts are gone and then look ups after that will show the new
ones, right?

Thanks.

--
tejun