xfs
[Top] [All Lists]

Re: [PATCH] xfs_fsr: more selinux fixes

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH] xfs_fsr: more selinux fixes
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 12 Nov 2015 09:50:25 -0600
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20151112131242.GB5068@xxxxxxxxxxxxxxx>
References: <5643B8B7.9030708@xxxxxxxxxxx> <20151112131242.GB5068@xxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

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@xxxxxxxxxx>
>> ---
>>
>> 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

<Prev in Thread] Current Thread [Next in Thread>