linux-ext4 - Re: [PATCH e2fsprogs-next] Fix extent flag validity tests in pass1 on big endian boxes.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 24 Mar 2008 18:32:51 -0600
From:	Andreas Dilger <adilger@....com>
To:	Eric Sandeen <sandeen@...hat.com>
Cc:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH e2fsprogs-next] Fix extent flag validity tests in pass1	on
 big endian boxes.

On Mar 24, 2008  17:13 -0500, Eric Sandeen wrote:
> Extent data is shared with the i_block[] space in the inode,
> but it is always swapped on access, not when the inode is read.
> 
> In e2fsck/pass1.c we must be careful when checking validity
> of the extents flag on the inode.  If the flag was set when
> the inode was read & swapped, then the extents data itself
> (in ->i_block[]) was NOT swapped, so testing for a valid
> extent header requires some swapping first.  Then, if we
> ultimately set the extents flag, all of i_block[] must be
> re/un-swapped.

This seems pretty awkward for any other users of the library.  Having the
i_block[] array NOT be swabbed if it is an extent file means that every
place in the code which is accessing this array also needs to do the
swabbing itself.  This would break the abstraction that the in-memory
inode is in host-endian order, and also forces every application to
understand the difference between extent- and non-extent-mapped inodes,
and the on-disk byte order.  Ugh.

IMHO, it would be better to swab the i_block[] array in
ext2fs_swap_inode_full() if the EXTENTS_FL is set, and in the rare
case of e2fsck needing to clear that flag then it should un-swap (if
needed), clear the flag, and re-swap (if needed).  This will very rarely
happen, I think.  Note that the Lustre extents patches did NOT do the
swap+clear_swap operation when clearing the extents flags because we
don't have any big-endian server systems (our PPC testing is limited to
userspace "make check"), though I think that is the right thing to do.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html