Re: [PATCH 0/5 v3] fs: Fixes for removing xid bits and security labels

[Jan Kara <jack@suse.cz>]

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 <jack@suse.cz>
SUSE Labs, CR