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]

Message-ID: <CA+55aFytvTPdBsv6BwSomUGWXZ_tKP=BxaeJ7FXBft0JNnjBCQ@mail.gmail.com>
Date:	Mon, 3 Feb 2014 13:03:32 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	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>,
	Al Viro <viro@...iv.linux.org.uk>,
	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 3, 2014 at 2:29 AM, Christoph Hellwig <hch@...radead.org> wrote:
> On Thu, Jan 30, 2014 at 02:01:38PM -0800, Linus Torvalds wrote:
>> In the end, all the original call-sites should have a dentry, and none
>> of this is "fundamental". But you're right, it looks like an absolute
>> nightmare to add the dentry pointer through the whole chain. Damn.
>
> It's not just ceph.  9p fundamentally needs it and I really want to
> convert 9p to the new code so that we can get rid of the lower level
> interfaces entirely and eventually move ACL dispatching entirely
> into the VFS.

Hmm. I spent some time actually seeing how painful it would be, and I
think we can do it.

The attached patch seems to work for me - it does *not* make any
actual semantic changes, but it pushes the dentry pointer into the
filesystems for the permission checking, and the
vfs_{create,mkdir,mknot,symlink,link,rmdir,unlink,rename} helper
functions.

NOTE! It does *not* actually push them into the ACL functions, and in
fact it doesn't even push it down to "generic_permission()" yet. But
it would be a prerequisite for doing so.

And interestingly, replacing the inode pointer with a dentry pointer
ended up actually simplifying several of the call-sites, so while
doing this patch I got the feeling that this was the better interface
anyway, and we should have done this long ago.

Now, to be honest, pushing it down one more level (to
generic_permission()) will actually start causing some trouble. In
particular, gfs2_permission() fundamentally does not have a dentry for
several of the callers.

But aside from being pretty large, this patch looks pretty good to me.
And as I mentioned, I suspect it would actually be the right thing to
do even if we don't go any futher. And modulo gfs2, it would make it
trivial to push down the dentry pointer to generic_permission and then
to the acl lookup code.

What do you think? I guess this patch could be split up into two: one
that does the "vfs_xyz()" helper functions, and another that does the
inode_permission() change. I tied them together mainly because I
started with the inode_permission() change, and that required the
vfs_xyz() change.

                       Linus

View attachment "patch.diff" of type "text/plain" (67416 bytes)