Re: [PATCH] fs: call security_d_instantiate in d_obtain_alias

[Dave Quigley]

On 11/19/2010 11:42 AM, J. Bruce Fields wrote:
> On Fri, Nov 19, 2010 at 12:28:09AM -0500, David Quigley wrote:
> > [snip]
> > > If you have persistent xattr support we need the dentry since the xattr
> > > code requires a dentry.  I have no idea why but that's what
> > > inode->i_op->getxattr() requires.
> > The original reason that the xattr operations take dentries is
> > because of p9fs and CIFS. CIFS uses the name of the file to grab the
> > extended attributes and so does p9fs. I had tried to remove this a
> > while ago but couldn't find a way around that.
> Both CIFS and FUSE are NFS-exportable, so both allow lookup by
> filehandle, so neither can count on getting a filename at this point.
>
> So, out of curiosity, do we know what will happen when selinux asks one
> of them for an xattr on a DCACHE_DISCONNECTED dentry?

SELinux uses several methods to determine file labeling. In the policy 
filesystems such as xfs and the ext* series of filesystems are marked as 
fs_use_xattr. In this process the file label is pulled from the 
security.selinux xattr on disk. However CIFS and FUSE (and NFS but our 
Labeled NFS changes are trying to fix this) all have the filesystem 
marked as genfs. When mounting the filesystem the fs_type is looked at 
to determine its labeling type. Since its genfs we lookup what label was 
determined to be the default for that filesystem type. In NFS's current 
state all NFS mounts regardless of version get the uniform label of 
nfs_t for everything listed on an nfs mount point. We have a similar 
situation for cifs and fuse. So in this case SELinux should not be 
asking for the security.selinux xattr from these file systems. However 
if a getxattr call to the security.selinux xattr is made on these 
filesystems it will still work I might be wrong but my understanding is 
just the a dentry in the DCACHE_DISCONNECTED state is not negative but 
just isn't in the tree anymore. So looking at vfs_getxattr I had made 
some modifications a while back to it. Assuming we have permissions to 
access the file determined by xattr_permission and 
security_inode_getxattr we check to see if it is in the security 
namespace.  If it is in the security namespace we call xattr_getsecurity 
which will attempt to get the security label from the inode first 
(security_inode_getsecurity). Because the convention is to call 
d_instantiate on inode create this should always work assuming an LSM is 
loaded. If it fails and we don't have an lsm loaded we fall back to 
checking the getxattr i_op and if that doesn't exist we fail with 
EOPNOTSUPP. That is what should happen on the getxattr call. I don't 
know if something is happening higher up that makes it so we never get 
to vfs_getxattr in the event that the dentry is in the 
DCACHE_DISCONNECTED state. If the DENTRY is disconnected though I'm not 
sure how getxattr from userspace would be able to have access to it 
except through a different name in the namespace.

> > When trying to find a
> > solution I also got push back from Miklos (FUSE) as he views a
> > filesystem being able to make xattr decisions based on the path name
> > being a valid use-case.
> So selinux may initialize an inode differently depending on which
> pathname it happened to be looked up under first?
>
> Factoring the name into the xattr return sounds scary to me.

The only current use of determining file label from path name is the 
situation that Eric Paris described with proc. I personally don't agree 
with miklos that the path to the xattr should make it return different 
information (unless im understanding him wrong). However the same thing 
is at work for CIFS as it exposes the windows alternate file streams 
which are accessed by adding the stream name to the end of the filename 
with a separator which I can't remember at the moment. If it was the 
situation that two fuse files shared the same inode and the 
security.selinux xattr was filled differently if it was accessed via 
/fuse/foo and /fuse/bar then yes the situation you described might 
happen. Normally this isn't a problem because file systems don't take 
the path into account so a hardlink to the same inode will still obtain 
the same security label. In reality the xattr is a piece of inode 
metadata and not a piece of dentry metadata.

> --b.

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