issues Ext4 inode flags

[Pedro Giffuni]

On 17.01.2014 08:40, Bruce Evans wrote:
> On Thu, 16 Jan 2014, Pedro Giffuni wrote:
>
>> I have been working around some issues in our ext2/3/4 support and
>> there is this problem:
>>
>> Before r260545 we were passing the Ext2/3/4 flags with some
>> conversion into the inode i_flags. Since our system flags don't
>> match the linux flags this actually introduced a lot of garbage.
>>
>> r260545 cut the garbage completely but it also does not pass
>> the EXT4 flags of which we need two for our ext4 ro implementation:
>> EXT4_EXTENTS and EXT4_INDEX. We also use EXT4_HUGE_FILE
>> but we can read that directly from the dinode.
>>
>> The flags are pretty specific to Ext4 so it doesn't make sense to add
>> them to to our stat.h and include them in the inode conversion routines.
>
> These flags seem to be unrelated to the ones controlled by chflags(2).
> ext*fs just packs them into the same word.
>
>> I have two options:
>>
>> 1- Pass only the flags that we need and clear the rest.
>> Obviously a hack, this seems this is somewhat safer and has
>> worked so far as it only applies to the read-only ext4. There is
>> still some space for collision:
>> EXT4_INDEX     --> UF_READONLY
>> EXT4_EXTENTS --> UF_HIDDEN
>>
>> The patch is here:
>> http://people.freebsd.org/~pfg/patches/ext2fs/ext2fs-passinode.patch
>
> There is plenty of space to spare, so don't use existing file flags or
> return the non-file-flags as garbage in the file flags word.  This 
> amounts
> to using a new field, except masking out these flags in 
> ext2fs_get/setattr()
> is not so automatic.  Only the masking in ext2fs_getattr() is very easy.
>

Ah yes. I can set up some private flags.
I am never ever writing EXT4_* flags to the dinode because we don't 
support ext4 in write mode so I don't need to set them at all, just 
check if they exist. I will do that, thank you.

>> 2- Create a field in the inode specifically to carry the Ext4_* inode 
>> flags.
>> This. of course, costs memory for a feature that is rarely used but
>> is cleaner and may be useful if we ever add write support.
>
> Miscellaneous non-file-flags can also be stored in i_flag.
>
> BTW, ntfs was much more broken than ext2fs here.  It confused i_flag with
> the file flags i_flags.  It never supported file flags, but returned the
> garbage in i_flag as the file flags.  i_flag contains mainly the highly
> volatile flags IN_MODIFIED, etc., but ntfs didn't support writing so it
> never set these.  It used i_flag for a couple of ntfs-specific flags
> whose values start a bit above IN_MODIFIED.  These numbers are still
> small, so the conflicted with numbers for file flags.
>

FWIW, We still have some Windows-related inode flags in stat.h.They do 
seem more pertinent for system utilities than the EXT4_* flags though.

> BTW2, comparison of ext2fs_setattr() with ufs_setattr() for flags setting
> shows some minor bugs:

Thanks for providing more tasks, just when I thought I was wrapping up ;).

Pedro.

> - ext2fs_setattr() is missing an inode update after changing the file
>   flags
> - both have a too-verbose comment about how securelevel governs changing
>   file flags.  The one in ext2fs has rotted, so it is now incomplete
>   (it is missing details about how securelevel itself is governed by
>   the security.jail.chflags_allowed sysctl).  The one in ffs has this
>   detail, so it is even more verbose.  Details about the restrictions
>   imposed by priv_check_cred(... PRIV_CHECK_SYSFLAGS ...) don't belong
>   in individual file systems!  This API exists partly to hide the 
> details.
>   However, this is misdocumented even worse in other places:
>   - the security.jail.chflags allowed sysctl is not documented in any
>     man page.
>   - the secuity(7) man page also doesn't mention any sysctl in the
>     security tree, or even "jail"
>   - the chflags(2) man page mentions securelevel.  It refers to init(8)
>     for details, but also gives some details (mainly in descriprions of
>     error numbers).  These details are now incomplete.  It is difficult
>     to give all the details without repeating too much (this man page
>     now tries to repeat all the details for 2 cases of [EPERM]) or
>     being too general to be useful (like POSIX saying "appropriate
>     privilege" where appropriateness is very system-dependent and
>     rarely documented).
>   - the pointer to init(8) is stale.  init(8) no longer contains many
>     but refers to securelevel(7) for them.  It gives a few details.  
> These
>     are incomplete as usual.
>   - the chflags(1) man page refers to securelevel(7) for all the details.
>
> Bruce