From owner-svn-src-all@FreeBSD.ORG Mon Jan 19 11:09:33 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1E92B10656D6; Mon, 19 Jan 2009 11:09:33 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail11.syd.optusnet.com.au (mail11.syd.optusnet.com.au [211.29.132.192]) by mx1.freebsd.org (Postfix) with ESMTP id A13258FC2A; Mon, 19 Jan 2009 11:09:32 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-107-120-227.carlnfd1.nsw.optusnet.com.au (c122-107-120-227.carlnfd1.nsw.optusnet.com.au [122.107.120.227]) by mail11.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n0JB9Slj028342 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 19 Jan 2009 22:09:29 +1100 Date: Mon, 19 Jan 2009 22:09:28 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Stanislav Sedov In-Reply-To: <20090118174357.3ed1459d.stas@FreeBSD.org> Message-ID: <20090119202452.X37158@delplex.bde.org> References: <200901181404.n0IE4uXw075698@svn.freebsd.org> <20090118140644.GT48057@deviant.kiev.zoral.com.ua> <20090118174357.3ed1459d.stas@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Kostik Belousov , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r187395 - head/sys/gnu/fs/ext2fs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Jan 2009 11:09:41 -0000 On Sun, 18 Jan 2009, Stanislav Sedov wrote: > Kostik Belousov 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 fails - 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). Bruce