svn commit: r187395 - head/sys/gnu/fs/ext2fs

Bruce Evans brde at
Mon Jan 19 03:09:41 PST 2009

On Sun, 18 Jan 2009, Stanislav Sedov wrote:

> Kostik Belousov <kostikbel at> mentioned:
>> Please see a discussion on the fs@ and reasoning why I declined to commit
>> the similar patch.

> ...
> The extra size added in inodes are used to store additional info like extended
> attributes, ACLs and so on. Each inode now has a field just after the legacy
> inode format struct that shows hom much additional space has been added to
> this particular inode. By analyzing this field the operating system can interpret
> additional data located in inode, if it understand its format (there might be
> application and/or OS specific data too).
> Our implementation just ignore this additional fields after sizeof(ext2fs_inode),
> both while reading and writing. Thus we don't interet this data yet we don't
> overwrite it.

If it does this, then it is quite broken, since the garbage data bites
implementations that _do_ interpret the data.  Writing to the unsupported
fields is especially important for either destruction or creation, so that
the garbage doesn't leak to new files when a disk inode is reused.

The field[s] showing how the additional space is used actually seem to be
_inside_ the legacy inode (*).  However, this doesn't seem to help, since
we seem to never touch them there either (we bzero() new inodes in
ext2_vget() but never bzero() the disk inode).  There seem to be old
bugs in this area.  The untouched fields are:
- osd1 (sic)  I don't understand this
- i_file_acl  leaking this would security holes
- i_dir_acl   leaking this would security holes
               this is abused for the high 32 bits of the file size in the
 	      case of regular files.  We use it in this case, but a few
 	      years ago when we didn't use it, leaking it would have
 	      given wrong file sizes.
- i_faddr     I don't understand this
_ osd2 (sic)  this has mainly things like an extra 16 bits for the the uid
               and gid.  Leaking these would give security holes more often
 	      than for acls.

I think there is no actual problem with at least the file acls.  What is
supposed to happen is:
- if the file system has any nonzero acls, then ext[23]fs under linux has
   set EXT2_FEATURE_COMPAT_EXT_ATTR in the superblock to indicate that
   this feature is used
- FreeBSD ext2fs doesn't support this feature, so mounting such file systems
- an ext2fs utility should offer to clear EXT2_FEATURE_COMPAT_EXT_ATTR in the
   superblock.  When it clears it there, it must clear all the associated
   metadata too.  Thus the metadata doesn't leak.
- hopefully there are compat flags for all the other extensions too.  There
   is one for using the file size extension.  I can't find one related to
   larger disk inodes.

(*) In linux-2.6.10, the declaration of struct ext3_inode is lexically
     identical to struct ext2_inode except for one s/2/3/ and apparent
     style bugs in the latter (__u16/32 hasn't been converted to __le16/32
     in new fields in the latter; this is only a style bug provided the
     new fields are ignored on read and zeroed on write).


More information about the svn-src-head mailing list