Re: [PATCH] xfs_fsr: more selinux fixes

Eric Sandeen
2015-11-12T15:50:29Z

On 11/12/15 7:12 AM, Brian Foster wrote:
> On Wed, Nov 11, 2015 at 03:52:55PM -0600, Eric Sandeen wrote:
>> Commit:
>>
>> 1adfe5c xfs_fsr: fix SWAPEXT failures under selinux
>>
>> attempted to fix up the fork offset under selinux, where
>> the temp file is created with a local attribute, but the
>> target file has remote attributes; this can lead to a smaller
>> data area in the temp inode, without enough room to swap extents
>> from the target inode. I remedied this by pushing the temp
>> file attribute to remote, but *only* if the target file's attr
>> was also remote.
>>
>> However, I have a case from the field where the parent dir
>> and the target file both have a context of:
>>
>> system_u:object_r:samba_share_t:s0
>>
>> but new files created in the dir have a context of
>>
>> unconfined_u:object_r:samba_share_t:s0
>>
>> This means the temp file has a smaller forkoff, and less space
>> in the inode for data, so we fail to swap the extents between
>> the two, because they don't fit.
>>
>> The following patch fixes this by allowing xfs_fsr to
>> kick the tempfile's attr out of local format even if the target
>> file's attr is local, if this will move the forkoff in the right
>> direction. This does pass all our fsr xfstests, though I'm not
>> sure we have any real coverage of fsr under selinux...
>>
>> The only functional change is the test at the very end of the
>> patch; the rest is comments, ascii art, and removing the
>> now-extraneous XFS_IOC_FSGETXATTRA ioctl.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
>> index c8ef18f..68b9819 100644
>> --- a/fsr/xfs_fsr.c
>> +++ b/fsr/xfs_fsr.c
> ...
>> /*
>> - * If target attr area is less than the temp's (diff < 0)
>> - * and the target is not local, write a big attr to
>> - * the temp file to knock the attr out of local format,
>> - * to match the target. (This should actually *increase*
>> - * the temp file's forkoffset when the attr moves out
>> - * of the inode)
>> + * If target attr area is less than the temp's
>> + * (diff < 0), write a big attr to the temp file to knock
>> + * the attr out of local format.
>> + * (This should actually *increase* the temp file's
>> + * forkoffset when the attr moves out of the inode)
>> */
>> - if (diff < 0 && fsx.fsx_nextents > 0) {
>> + if (diff < 0) {
>
> Space before tab issue on the line above. Looks fine otherwise, but I
> wonder if it would also be a good idea to add an informative fsrprintf()
> here if we proceed when (fsx.fsx_nextents > 0)?

I can add a debug message, sure - I'm not sure when it would be useful, but
we have plenty of other fiddly messages in there about what action is taken.

Thanks,
-Eric