linux-kernel - Re: [PATCH v2] ceph: fix posix ACL hooks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 3 Feb 2014 21:59:08 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Christoph Hellwig <hch@...radead.org>,
	Ilya Dryomov <ilya.dryomov@...tank.com>,
	Sage Weil <sage@...tank.com>, Dave Jones <davej@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	ceph-devel@...r.kernel.org,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Guangliang Zhao <lucienchao@...il.com>,
	Li Wang <li.wang@...ntykylin.com>, zheng.z.yan@...el.com
Subject: Re: [PATCH v2] ceph: fix posix ACL hooks

On Mon, Feb 03, 2014 at 01:03:32PM -0800, Linus Torvalds wrote:

> -	err = vfs_mkdir(path.dentry->d_inode, dentry, mode);
> +	err = vfs_mkdir(path.dentry, dentry, mode);

Pointless - path.dentry == dentry->d_parent anyway.
 
> -	err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
> +	err = vfs_mknod(path.dentry, dentry, mode, dev->devt);

Ditto.

> @@ -237,7 +237,7 @@ static int dev_rmdir(const char *name)
>  		return PTR_ERR(dentry);
>  	if (dentry->d_inode) {
>  		if (dentry->d_inode->i_private == &thread)
> -			err = vfs_rmdir(parent.dentry->d_inode, dentry);
> +			err = vfs_rmdir(parent.dentry, dentry);
 
 Ditto, with s/path/parent/
> @@ -324,7 +324,7 @@ static int handle_remove(const char *nodename, struct device *dev)
>  			mutex_lock(&dentry->d_inode->i_mutex);
>  			notify_change(dentry, &newattrs, NULL);
>  			mutex_unlock(&dentry->d_inode->i_mutex);
> -			err = vfs_unlink(parent.dentry->d_inode, dentry, NULL);
> +			err = vfs_unlink(parent.dentry, dentry, NULL);
>  			if (!err || err == -ENOENT)
>  				deleted = 1;
>  		}
 
And here as well.
 
> +++ b/drivers/staging/lustre/lustre/lvfs/lvfs_linux.c
> @@ -222,8 +222,8 @@ int lustre_rename(struct dentry *dir, struct vfsmount *mnt,
>  	if (IS_ERR(dchild_new))
>  		GOTO(put_old, err = PTR_ERR(dchild_new));
>  
> -	err = ll_vfs_rename(dir->d_inode, dchild_old, mnt,
> -			    dir->d_inode, dchild_new, mnt, NULL);
> +	err = ll_vfs_rename(dir, dchild_old, mnt,
> +			    dir, dchild_new, mnt, NULL);


... and again, that's completely pointless.

> -int afs_permission(struct inode *inode, int mask)
> +int afs_permission(struct dentry *dentry, struct inode *inode, int mask)

Oh, _lovely_.  So not only do we pass dentry, the arguments are redundant
as well.

> -static inline int btrfs_may_create(struct inode *dir, struct dentry *child)
> +static inline int btrfs_may_create(struct dentry *parent, struct dentry *child)

I'm fairly sure that it's also pointless, because parent is going to be, well,
the parent.  Of child.

> +static int gfs2_vfs_permission(struct dentry *dentry, struct inode *inode, int mask)
> +{
> +	return gfs2_permission(inode, mask);
> +}

Er...  You do realize that callers of gfs2_permission() tend to have
the dentry in question, either directly or as ->d_parent of something
they have?


I really hate the whole thing... ;-/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/