[Pvfs2-developers] kernel module copy attrs race

[Sam Lang]

I've been debugging a problem with simul returning EACCES errors on  
open of a PVFS file.  It turns out the bug was due to permissions  
being (re-)set on an inode without a mutex locking the write.  This  
was causing the permissions checks in other opens of the same file to  
fail because the mode was wrong.

For each (subsequent) open, the kernel calls d_revalidate.  The PVFS  
implementation of d_revalidate does a lookup to verify that the handle  
of the inode and looked-up handle match, then it does a getattr on  
that handle, and copies the attributes into the inode.  This is where  
the race occurred, during copying of the attrs.  In pvfs2-utils.c:231,  
we set:

inode->i_mode = 0;

/* figure out the correct permissions for the file based on pvfs attrs  
*/

if (attrs->perms & PVFS_O_EXECUTE)
             perm_mode |= S_IXOTH;
if (attrs->perms & PVFS_O_WRITE)
             perm_mode |= S_IWOTH;
....

inode->i_mode |= perm_mode;

That entire block is unprotected, so between setting the i_mode to 0  
and then perm_mode, the i_mode can be accessed by other processes (in  
the kernel vfs permissions checking code, for example).

I think the problem is exacerbated by the large block of code that  
converts the pvfs attrs into the perm_mode field all while i_mode is  
0.  Just setting inode->i_mode = perm_mode directly made the bug  
disappear for my test case, even though there's still a RMW race there.

I'm tempted to throw a big mutex_lock(inode->i_mutex) around the  
entire code block that does copy_attrs_to_inode.  Any objections  
against doing that?

-sam


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2417 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20080325/7295eabf/smime.bin