[Ocfs2-devel] [RFC] The reflink(2) system call v4.

[Casey Schaufler]

Joel Becker wrote:
> On Tue, May 12, 2009 at 01:32:47PM -0400, Stephen Smalley wrote:
>   
>> On Tue, 2009-05-12 at 10:22 -0700, Joel Becker wrote:
>>     
>>> On Tue, May 12, 2009 at 08:18:34AM -0400, Stephen Smalley wrote:
>>>       
>>>> Is preserve_security supposed to also control the preservation of the
>>>> SELinux security attribute (security.selinux extended attribute)?  I'd
>>>> expect that either we preserve all the security-relevant attributes or
>>>> none of them.  And if that is the case, then SELinux has to know about
>>>> preserve_security in order to know what the security context of the new
>>>> inode will be.  
>>>>         
>>> 	Thank you Stephen, you read my mind.  In the ocfs2 case, we're 
>>> expecting to just reflink the extended attribute structures verbatim in
>>> the preserve_security case.
>>>       
>> And in the preserve_security==0 case, you'll be calling
>> security_inode_init_security() in order to get the attribute name/value
>> pair to assign to the new inode just as in the normal file creation
>> case?
>>     
>
> 	Oh, absolutely.
> 	As an aside, do inodes ever have more than one security.*
> attribute?

ACLs, capability sets and Smack labels can all exist on a file at
the same time. I know of at least one effort underway to create a
multiple-label LSM.

>   It would appear that security_inode_init_security() just
> returns one attribute, but what if I had a system running under SMACK
> and then changed to SELinux?  

The Smack attribute would hang around, it would just be unused.


> Would my (existing) inode then have
> security.smack and security.selinux attributes?
>   

Yup. It happens all the time. Whenever someone converts a Fedora
system to Smack they end up with a filesystem full of unused selinux
labels. It does no harm.

>   
>>>> Also, if you are going to automatically degrade reflink(2) behavior
>>>> based on the owner_or_cap test, then you ought to allow the same to be
>>>> true if the security module vetoes the attempt to preserve attributes.
>>>> Either DAC or MAC logic may say that security attributes cannot be
>>>> preserved.  Your current logic will only allow graceful degradation in
>>>> the DAC case, but the MAC case will remain a hard failure.
>>>>         
>>> 	I did not think of this, and its a very good point as well.  I'm
>>> not sure how to have the return value of security_inode_reflink()
>>> distinguish between "disallow the reflink" and "disallow
>>> preserve_security".  But since !preserve_security requires read access
>>> only, perhaps we move security_inode_reflink up higher and say:
>>>
>>> 	error = security_inode_reflink(old_dentry, dir);
>>> 	if (error)
>>> 		preserve_security = 0;
>>>
>>> Here security_inode_reflink() does not need new_dentry, because it isn't
>>> setting a security context.  If it's ok with the reflink, we'll be
>>> copying the extended attribute.  If it's not OK, it falls through to the
>>> inode_permission(inode, MAY_READ) check, which will check for plain old
>>> read access.
>>> 	What do we think?
>>>       
>> I'd rather have two hooks, one to allow the security module to override
>> preserve_security and one to allow the security module to deny the
>> operation altogether.  The former hook only needs to be called if
>> preserve_security is not already cleared by the DAC logic.  The latter
>> hook needs to know the final verdict on preserve_security in order to
>> determine the right set of checks to apply, which isn't necessarily
>> limited to only checking read access.
>>     
>
> 	Ok, is that two hooks or one hook with specific error returns?
> I don't care, it's up to the LSM group.  I just can't come up with a
> good distinguishing set of names if its two hooks :-)
>
> Joel
>
>