issue with unsetting 'arch' flag

[Garrett Cooper]

On Thu, Oct 7, 2010 at 12:43 PM, Sergey Kandaurov <pluknet at gmail.com> wrote:
> On 7 October 2010 22:45, Jaakko Heinonen <jh at freebsd.org> wrote:
>> On 2010-10-06, Alexander Best wrote:
>>> $ sudo rm -d /tmp/chflags.XXXXXX
>>> $ tmpfile=`mktemp /tmp/chflags.XXXXXX`
>>> $ sudo chflags arch $tmpfile
>>> $ chflags noarch $tmpfile
>>>
>>> is what's causing the problem. the last chflags call should fail, but it
>>> doesn't.
>>
>> Here is a patch for UFS:
>>
>> %%%
>> Index: sys/ufs/ufs/ufs_vnops.c
>> ===================================================================
>> --- sys/ufs/ufs/ufs_vnops.c     (revision 213507)
>> +++ sys/ufs/ufs/ufs_vnops.c     (working copy)
>> @@ -556,6 +556,9 @@ ufs_setattr(ap)
>>                            & (SF_NOUNLINK | SF_IMMUTABLE | SF_APPEND) ||
>>                            (vap->va_flags & UF_SETTABLE) != vap->va_flags)
>>                                return (EPERM);
>> +                       if ((ip->i_flags & SF_SETTABLE) !=
>> +                           (vap->va_flags & SF_SETTABLE))
>> +                               return (EPERM);
>>                        ip->i_flags &= SF_SETTABLE;
>>                        ip->i_flags |= (vap->va_flags & UF_SETTABLE);
>>                        DIP_SET(ip, i_flags, ip->i_flags);
>> %%%
>>
>> The patch has a potential to break something if someone assumes that
>> non-super-user can modify UF_SETTABLE flags with the SF_SETTABLE part
>> set to zero. However with a quick peek this seems to be what NetBSD
>> does.
>
> Just for reference:
> this comes from NetBSD PR kern/3491 and fixed before 1.3R.
> I just checked arch test, and it works as expected with the change.
> All chflags tests from fstest suite passed as well.

    This only potentially fixes UFS though -- not ext*, msdosfs, ntfs,
ZFS, etc, which that it can still fail the requirement in the
chflags(2) manpage that states:

     [EOPNOTSUPP]       The underlying file system does not support file
                        flags.

    Or should it be a different requirement, i.e. EINVAL for bogus values?
Thanks,
-Garrett