From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 0/5 v3] fs: Fixes for removing xid bits and security labels Date: Tue, 19 May 2015 17:05:42 +0200 Message-ID: <20150519150542.GA7158@quack.suse.cz> References: <1432028803-32296-1-git-send-email-jack@suse.cz> <555B467E.4020606@schaufler-ca.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jan Kara , Al Viro , Linus Torvalds , linux-fsdevel@vger.kernel.org, dchinner@redhat.com, Serge Hallyn , linux-security-module@vger.kernel.org To: Casey Schaufler Return-path: Content-Disposition: inline In-Reply-To: <555B467E.4020606@schaufler-ca.com> Sender: linux-security-module-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Tue 19-05-15 07:19:42, Casey Schaufler wrote: > On 5/19/2015 2:46 AM, Jan Kara wrote: > > Hello, > > > > This is a third version of my patches to fix handling of s[ug]id bits and > > capabilities xattrs in VFS. There are a few issues I found: > > > > 1) MS_NOSEC handling is broken - we set it after each file_remove_suid() call. > > However we needn't have removed suid bit simply because we have > > CAP_SYS_FSID and further writes to the file from processes without this > > capability still need to clear the suid bit. > > 2) file_remove_suid() is a misnomer since it also handles removing of > > security labels. It is even more confusing because should_remove_suid() > > doesn't return whether file_remove_suid() is needed or not. > > 3) On truncate we do clear suid bits but not security labels. According to > > documentation in include/linux/security.h that's a bug but please correct > > me if I'm wrong. > > The only security module that provides a hook for inode_killpriv is the > capability module. That clears the file based capabilities. Neither of the > modules that use inode based labels (SELinux and Smack) provide an inode_killpriv > hook. The text in lsm_hooks.h (moved from security.h) says "similar security > labels". The file based capabilities *are* similar to the setuid bit. The > mandatory access control labels used by SELinux and Smack are *dissimilar*. > The text in lsm_hooks should say "attributes", not "labels". > > So the code you have will work the way everyone wants it to. We just need > to stop saying "labels" when we mean something else. Sure, I'll update the changelogs to say 'file capabilities'. That should make things less confusing. Honza > > 4) ocfs2 doesn't clear capability xattrs - hard to fix, I left it alone for > > now. > > 5) XFS didn't provide proper exclusion for clearing mode bits. > > > > This series aims at fixing above issues. Al, can you please merge the > > patches? Thanks! > > > > Changes since v2: > > * Rebased on top of current Linus' tree > > * Improved patch 1 to use inode_has_no_xattr() as Linus suggested > > > > Changes since v1: > > * Removed bogus patch changing inode_set_flags() > > * Updated changelog of patch 4/5 to better explain why ->inode_killpriv > > should be called > > * Included a fix for MS_NOSEC handling in this series. > > > > Honza > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- Jan Kara SUSE Labs, CR