linux-ext4 - Re: [PATCH][7/28] e2fsprogs-extents.patch

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]

Message-id: <20080219043545.GJ3029@webber.adilger.int>
Date:	Mon, 18 Feb 2008 21:35:45 -0700
From:	Andreas Dilger <adilger@....com>
To:	Eric Sandeen <sandeen@...hat.com>
Cc:	"Theodore Ts'o" <tytso@....edu>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH][7/28] e2fsprogs-extents.patch

On Feb 18, 2008  11:56 -0600, Eric Sandeen wrote:
> > @@ -904,21 +910,75 @@ void e2fsck_pass1(e2fsck_t ctx)
> > +		eh = (struct ext3_extent_header *)inode->i_block;
> > +		if ((inode->i_flags & EXT4_EXTENTS_FL)) {
> > +			if ((LINUX_S_ISREG(inode->i_mode) ||
> > +			     LINUX_S_ISDIR(inode->i_mode)) &&
> 
> So this trips up on things like sockets, fifos, and block & char nodes.

Hrm, not impossible, since Lustre only uses extent-based filesystems for
regular file storage.

> Also this is unhappy:
> 
> > @@ -137,7 +141,7 @@ int e2fsck_pass1_check_device_inode(ext2
> >  	 * If the index flag is set, then this is a bogus
> >  	 * device/fifo/socket
> >  	 */
> > -	if (inode->i_flags & EXT2_INDEX_FL)
> > +	if (inode->i_flags & (EXT2_INDEX_FL | EXT4_EXTENTS_FL))
> >  		return 0;
> 
> Do we really care if these have the extents flag set?  IOW should we
> make sure the kernel doesn't set the flag, or should we make e2fsck not
> care...

The Lustre extents patches clear the EXT4_EXTENTS_FL always (i.e. they
are never set on directories) so we've never seen these problems.

> There are enough checks in e2fsck to show the intent was that these
> files should not have the extents flag set, but I'm not sure why it
> matters enough that the kernel needs to run around being sure to clear
> it....
> 
> Or... (rambling on now) it seems odd to me that zero-length files have
> the extents flag set at all; should we only set extents when we actually
> get a block allocated to the file?  That would also take care of this
> from the kernel side I think.

Yes, I'd be for e2fsck clearing this flag, but as I mentioned in the
concall, I think it is better to have the kernel just stop inheriting
all flags from the parent directory, or possibly just have a fixed
range of flags that are being propogated to child inodes.

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