[Unionfs] Re: 2.6.26-rc4: smack and unionfs deadlock

[Casey Schaufler]

Erez Zadok wrote:
> In message <484D4F96.2010104 at schaufler-ca.com>, Casey Schaufler writes:
>   
>> Luiz Fernando N. Capitulino wrote:
>>     
>>> | >  Casey, I'm afraid that only Erez (unionfs maintainer)
>>> | > can answer you here but it seems that he's away or
>>> | > something.
>>> | >
>>> | >  His last message on unionfs ML dates about one month
>>> | > back.
>>> | >
>>> | >  :(
>>> | Ok. Here's a temporary patch that makes everything except looking at
>>> | the Smack label work properly. I haven't signed it off because I
>>> | don't think it's ready for prime-time, but if the problem is holding
>>> | anyone up it will allow them to proceed.
>>>
>>>  Passed basic tests.
>>>
>>>  Does the problem with the Smack label only happen for unionfs
>>> mounts?
>>>   
>>>       
>> Unionfs is the only case were I've seen this problem,
>> but of course seeing it here leads me to think I really
>> ought to be looking hard for issues like this. I fear
>> that there may be locking or other behaviors that have
>> undesired consequences in nested file systems that I
>> honestly haven't gotten to trying.
>>     
>
> Hi Casey,
>
> Now that unionfs-2.4 is out, I'd like to look more closely at this issue.  I
> suspect the problem may not be confined to unionfs, but is probably
> applicable to layered file systems in general.  There's an important
> question when it comes to layering: how to do the security right.  The linux
> VFS, as it stands, wasn't really design for stacking.  Typical problems I've
> seen are:
>
> 1. The vfs performs some security/permission check, then calls the mounted
>    (stackable) file system.
>
> 2. The stackable file system, wanting to do the right, uses VFS methods such
>    as vfs_* and friends, but passing them objects (e.g., dentry, inode) of
>    the lower file system (e.g., ext2, tmpfs).
>
> The problems arise from this:
>
> - in order to avoid making the stackable f/s reproduce a lot of the vfs
>   code/state, it tries to call approved vfs_* methods.
>
> - but sometimes, those vfs_* methods include code (e.g, locking or
>   permission checking) that should only be done at the top of the stack, and
>   only once.  So if a stackable f/s calls said vfs_* method, it's
>   duplicating effort, complicating locking, or worse -- gets into a deadlock
>   (or lockdep warning).
>
> - at times, we (in unionfs) were forced to call a lower f/s op directly, not
>   through the vfs_* method.  eg., inode->i_op->function(args...).  There are
>   also cases where we have to temporarily elevate capabilities.
>
> With the introduction of stackable file systems, I think the VFS may need
> two kinds of callable methods: those that are applicable only to the top of
> the stack, and those that each layer can use safely on the lower layer.
>
> Anyway, I'm interested in helping to improve the VFS to better handle
> layering+security.  If you have some ideas, I'd love to hear them

The current problem shows up during inode instantiation. I want
to get the Smack label (I expect SELinux has the same trouble, BTW)
for the security blob, so I call i_op->getxattr(). This hangs due
to locking in the underlying file system. It is interesting to note
that the only operation that appears to have this behavior is
the getxattr, perhaps the other operations have been accommodated
in some way. The hack is to bypass the getxattr during instantiation.
Because the underlying file system is used during accesses the
access will behave as it should, and in fact the only issue is that
an attempt to look at the unionfs inode (stat, getxattr, ...) will
succeed where it shouldn't even though the data is properly protected.

I don't pretend to understand the underlying locking well enough to
explain why there is a problem.

Thank you.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.fsl.cs.sunysb.edu/pipermail/unionfs/attachments/20080803/6e2a4510/attachment-0001.htm