linux-kernel - Re: [git pull] vfs pile 1.5

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: <20110726161831.GA6704@infradead.org>
Date:	Tue, 26 Jul 2011 12:18:31 -0400
From:	Christoph Hellwig <hch@...radead.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Markus Trippelsdorf <markus@...ppelsdorf.de>,
	Steven Liu <lingjiujianke@...il.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [git pull] vfs pile 1.5

On Tue, Jul 26, 2011 at 09:10:02AM -0700, Linus Torvalds wrote:
> However, if we *do* have POSIX ACL's enabled, but we just have the XFS
> ACL's disabled, then just returning NULL from xfs_get_acl() is
> actually a horrible idea, because then the VFS code will always call
> that function and it never gets cached as "I have no ACL's". Which is
> why I didn't like the inline function approach. Well, at least not the
> one that only returns NULL - we also want to fill in the cache.

Even getting to check_acl() requires MS_POSIXACL to be set on the
superblock, which it won't be if CONFIG_XFS_ACL isn't set.

These days we can probably kill CONFIG_FOOFS_ACL given that everyone
now uses the generic code, and due to the deeper VFS interaction is
more or less forced to anyway.  Historically that wasn't the case.

> I'm actually getting more and more convinced that we should just do
> that "set_acl_cache()" in generic code for the "get_acl()" case. Even
> without any locking it's no worse than all the current filesystems,
> since they *also* do it with no locking. And then this kind of issue
> wouldn't come up.

While I agree with you that we should life it into the common code,
the common caching is not related at all to this particular issue.

The code that fails to compile is where XFS implements the default ACL
inheritance when creating new inodes.  Move filesystems do this a bit
simpler and dumber and do the ACL-induced i_mode modifications after
creating the inode. XFS tries to create the inode with the correct mode
from the first moment and thus has a slightly broader interface between
the "normal" inode ops and its ACL code.

Due to various interactions with the fs transaction code the inheritance
code is at the very end of my list of ACL pieces to move to common code.

--
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/