linux-ext4 - Re: [PATCH] clear extents flag on inodes created in ext4

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] [day] [month] [year] [list]

Message-ID: <47BB0E98.7090905@redhat.com>
Date:	Tue, 19 Feb 2008 11:15:04 -0600
From:	Eric Sandeen <sandeen@...hat.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
CC:	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] clear extents flag on inodes created in ext4_mknod

Aneesh Kumar K.V wrote:

>> How about enabling it only for directory and regular files rather than
>> enabling it globally and then disabling the flag for  symlink and device
>> files ?
>>
> 
> how about something like below. There are two reason for not inheriting
> the i_flag from directory.

Looks about right to me... see below

> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index d2c2e55..f430939 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -794,7 +794,12 @@ got:
>  	ei->i_dir_start_lookup = 0;
>  	ei->i_disksize = 0;
>  
> -	ei->i_flags = EXT4_I(dir)->i_flags & ~EXT4_INDEX_FL;
> +	/*
> +	 * Don't inherit extent flag from directory. We set extent flag on
> +	 * newly created directory and file only if -o extent mount option is
> +	 * specfied
 nitpick:  ^^^^ typo

> +	 */
> +	ei->i_flags = EXT4_I(dir)->i_flags & ~ (EXT4_INDEX_FL|EXT4_EXTENTS_FL);
>  	if (S_ISLNK(mode))
>  		ei->i_flags &= ~(EXT4_IMMUTABLE_FL|EXT4_APPEND_FL);
>  	/* dirsync only applies to directories */
> @@ -836,8 +841,10 @@ got:
>  		ext4_std_error(sb, err);
>  		goto fail_free_drop;
>  	}
> -	if (test_opt(sb, EXTENTS) && !S_ISLNK(mode)) {
> -		EXT4_I(inode)->i_flags |= EXT4_EXTENTS_FL;
> +	if (test_opt(sb, EXTENTS)) {
> +		/* set extent flag only for diretory and file */
> +		if (S_ISDIR(mode) || S_ISREG(mode))
> +			EXT4_I(inode)->i_flags |= EXT4_EXTENTS_FL;
>  		ext4_ext_tree_init(handle, inode);
>  		err = ext4_update_incompat_feature(handle, sb,
>  						EXT4_FEATURE_INCOMPAT_EXTENTS);

Hm you only set the extents flag for dir & reg, but you call the next
two functions for all files?  I think the next 2 lines need to be part
of the conditional, no? (and then there can be just one big test,
indentation can come out a bit...?)

But, the spirit of the patch looks right.

-Eric



> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 23902ba..da942bc 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1771,7 +1771,6 @@ retry:
>  #ifdef CONFIG_EXT4DEV_FS_XATTR
>  		inode->i_op = &ext4_special_inode_operations;
>  #endif
> -		EXT4_I(inode)->i_flags &= ~EXT4_EXTENTS_FL;
>  		err = ext4_add_nondir(handle, dentry, inode);
>  	}
>  	ext4_journal_stop(handle);

-
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