Re: [PATCH 2/5] binderfs: prevent renaming the control dentry

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Jan 18, 2019 at 03:53:41PM +0100, Christian Brauner wrote:
> We don't allow to unlink it since it is crucial for binderfs to be useable
> but if we allow to rename it we make the unlink trivial to bypass. So
> prevent renaming too and simply treat the control dentry as immutable.
> 
> Take the opportunity and turn the check for the control dentry into a
> separate helper is_binderfs_control_device() since it's now used in two
> places.
> Additionally, replace the custom rename dance we did with call to
> simple_rename().

Umm...

> +static inline bool is_binderfs_control_device(const struct inode *inode,
> +					      const struct dentry *dentry)
> +{
> +	return BINDERFS_I(inode)->control_dentry == dentry;
> +}

What do you need an inode for?

static inline struct binderfs_info *BINDERFS_I(const struct inode *inode) 
{
        return inode->i_sb->s_fs_info;
}

so it looks like all you care about is the superblock.  Which can be
had simply as dentry->d_sb...

Besides, what's the point of calling is_binderfs_device() in ->rename()?
If your directory methods are given dentries from another filesystem,
the kernel is already FUBAR.  So your rename should simply do
	if (is_binderfs_control_device(old_dentry) ||
	    is_binderfs_control_device(new_dentry))
		return -EPERM;
	return simple_rename(......);
and that's it...