Re: New reflink(2) syscall

[Stephen Smalley <sds@tycho.nsa.gov>]

On Tue, 2009-05-05 at 01:35 +1000, James Morris wrote:
> On Mon, 4 May 2009, Stephen Smalley wrote:
> 
> [added fsdevel to this thread..]
> 
> > > 
> > > http://marc.info/?l=linux-fsdevel&m=124133134306871&w=2
> > > http://marc.info/?l=linux-fsdevel&m=124133137106901&w=2
> > > http://marc.info/?l=linux-fsdevel&m=124133134406874&w=2
> > > 
> > > We need to determine if the security hooks included are appropriate, and 
> > > provide feedback (I've asked the author to cc this list with future 
> > > postings).
> > > 
> > > In summary, reflink(2) has an interface similar to link(2), but creates a 
> > > new file with copy on write semantics.
> > > 
> > > The existing LSM hooks are security_path_mknod() and 
> > > security_inode_create(), as well as security_inode_permission() via 
> > > may_create().
> > > 
> > > For SELinux, at least, it seems we may need another check to control 
> > > information flow from the source file to the new file, which may be 
> > > instantiated with a different security context.
> > 
> > The reflink(2) documentation in patch 1/3 suggests that the security
> > context of the new file would be initially identical to the original
> > file ("All file attributes and extended attributes of the new file must
> > be identical to the source file...").  
> 
> Ok (I missed the extended attribute mention initially).
> 
> > In that case, security_inode_create() is not the right hook to use as
> > security_inode_create() presumes that the new file is labeled based on
> > the creating process and the parent directory and that the filesystem
> > will use the security attribute name:value pair returned by
> > security_inode_init_security() as the initial attribute for the new
> > file.
> > 
> > It sounds like reflink(2) is more akin to copying a file while
> > preserving attributes (ala cp -a foo bar), only performing the actual
> > copying lazily.  In the case of a normal file copy while preserving
> > attributes, we would check that the process can open and read the
> > original file, write to the target directory, create a file with the
> > security context of the original file, and set the mode/owner/timestamps
> > of the new file.  That sequence of checks however is based on the
> > presumption that the data flows through the process, potentially being
> > mutated by it, and that we don't directly see the inter-file
> > relationship in the kernel.  With direct kernel support for file 
> > copying, we may want a different set of checks. 
> 
> What's fundamentally different, though, that the process would only be 
> able to then modify the data in a subsequent syscall?

Since the data doesn't flow through the process at all, it can neither
be leaked nor modified by the process.  Whereas normally the data would
be copied into the memory of the process (and potentially leaked
elsewhere) and the process could write any arbitrary data it liked to
the new file.  As a result, one might be willing to allow reflink(2) in
situations where one would not be willing to allow a userspace file
copy.

> > I think we likely need a new security hook.
> 
> Agreed, perhaps something like:
> 
> int security_inode_reflink(struct dentry *dentry, struct inode *dir);

I'd pass the same arguments as vfs_reflink(), i.e. old_dentry, dir,
new_dentry.

> > BTW, the DAC permission checking here also needs some thought, and 
> > wouldn't be handled by the LSM hook.  Should reflink(2) require DAC read 
> > permission to the file, like a file copy would?
> 
> Yes -- it seems all you need to be able to do at the moment is lookup the 
> original file for the syscall to work from that end.
> 
> > And if the owner of the original differs from the fsuid of the current 
> > process, should reflink(2) require CAP_CHOWN in order to set the 
> > ownership of the copy to the original's owner?
> 
> Good question :-)
> 
> Also, do we ignore create_sid in SELinux for this?

Yes (assuming attribute preservation by the filesystem).

-- 
Stephen Smalley
National Security Agency