On Wed, Jun 16, 2010 at 09:07:10AM -0400, Dan Rosenberg wrote: > Sure thing. This patch is against 2.6.34, but it appears that it can > apply to >= 2.6.25. Let me know if you need a fix for < 2.6.25. > > For those new to the conversation, this patch prevents user "foo" from > using the SWAPEXT ioctl to swap a write-only file owned by user "bar" > into a file owned by "foo" and subsequently reading it. It does so by > checking that the file descriptors passed to the ioctl are also opened > for reading. This part is okay. If you provide a Signed-off-by: line it can be put into the tree ASAP. > In addition, after swapping any suid/sgid bits should be > cleared. I'm still trying to understand this one. Clearly we do not want to simply drop the suid/sgid bits here - swapext is just supposed to optimize file layout, but not change i_mode. So if the suid bit really is a risk here we need to refuse to swapext it. What's the scenario here: - swapext is called by the owner and the suid bit is set, or the owner is member of the group and the sgid bit is set, this should be fine. - the caller is not the owner, but has write access to the file. an actual write to change the data by the user would already drop the bits, so in theory swapext should be fine. But we might lose some important metadata in extended attributes that previously might have made the file safe, say per-file capabilities. I'd love to here the exact scenario, but for let's play safe and refuse the swapex for that case, by doing something: /* * SGID without any exec bits is just a mandatory locking mark. */ #define is_sgid(mode) \ (((mode) & (S_ISGID|S_IXGRP)) == (S_ISGID|S_IXGRP)) ... struct inode *inode, *tmp_inode; ... inode = file->f_path.dentry->d_inode; tmp_inode = tmp_file->f_path.dentry->d_inode; ... error = XFS_ERROR(EPERM); if ((inode->i_mode & S_ISUID) && !is_owner_or_cap(inode)) goto out; if ((tmp_inode->i_mode & S_ISUID) && !is_owner_or_cap(tmp_inode)) goto out; if (is_sgid(inode->i_mode) && !in_group_p(inode->i_gid)) goto out; if (is_sgid(tmp_inode->i_mode) && !in_group_p(tmp_inode->i_gid)) goto out; _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
