[Ocfs2-tools-devel] [PATCH 1/1] Add corrupt code for inline data in fswreck

Tao Ma tao.ma at oracle.com
Mon Aug 11 07:10:06 PDT 2008


Hi tristan,
	Thanks for your work on this.

First, please read http://lxr.linux.no/linux/Documentation/CodingStyle. 
It explains the policy for writing Linux kernel source code. Although 
ocfs2-tools isn't a kernel project, I think we should follow the same 
rule. Actually there is a good tool which will help you find the coding 
style problem, and you can find in linux-kernel-src/scripts/checkpatch.pl.

Some other comments are below.
tristan wrote:
>  Add corrupt code for inline data in fswreck,include both regular file
> and dir.
>  Mainly aims at corrupting the newly added checking codes in
> fsck.ocfs2.checks for inline-data.
>  Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> 
> ---
>  fswreck/corrupt.c           |   13 +++++
>  fswreck/dir.c               |  110
> +++++++++++++++++++++++++++++++++++++++++++
>  fswreck/include/dir.h       |    2 +
>  fswreck/include/fsck_type.h |   12 ++++-
>  fswreck/include/inode.h     |    2 +
>  fswreck/include/main.h      |    5 ++-
>  fswreck/inode.c             |  105
> +++++++++++++++++++++++++++++++++++++++++
>  fswreck/main.c              |    4 ++
>  8 files changed, 251 insertions(+), 2 deletions(-)
> 
> diff --git a/fswreck/corrupt.c b/fswreck/corrupt.c
> index f698cb0..188b767 100644
> --- a/fswreck/corrupt.c
> +++ b/fswreck/corrupt.c
> @@ -147,6 +147,19 @@ void corrupt_file(ocfs2_filesys *fs, int code,
> uint16_t slotnum)
>  	case CORRUPT_DIR_NOT_CONNECTED:
>  		func = mess_up_dir_not_connected;
>  		break;
> +	case CORRUPT_REGULAR_INLINE_DATA_FLAG:
> +		func = mess_up_regular_inline_flag;
> +		break;
> +	case CORRUPT_REGULAR_INLINE_DATA_SIZE:
> +		func = mess_up_regular_inline_size;
> +		break;
> +	case CORRUPT_DIR_INLINE_DATA_FLAG:
> +		func = mess_up_dir_inline_flag;
> +		break;
> +	case CORRUPT_DIR_INLINE_DATA_SIZE:
> +		func = mess_up_dir_inline_size;
> +		break;
> +
>  	default:
>  		FSWRK_FATAL("Invalid code=%d", code);
>  	}
> diff --git a/fswreck/dir.c b/fswreck/dir.c
> index 3338a17..34fdae9 100644
> --- a/fswreck/dir.c
> +++ b/fswreck/dir.c
> @@ -452,3 +452,113 @@ void mess_up_dir_not_connected(ocfs2_filesys *fs,
> uint64_t blkno)
>  
>  	return;
>  }
> +
> +void create_inline_dir(ocfs2_filesys *fs,const char *dirname,
> +		       uint64_t blkno, uint64_t *retblkno)
> +{
> +	errcode_t ret;
> +	uint64_t tmp_blkno;
> +
> +	ret = ocfs2_check_directory(fs,blkno);
> +	if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +	
> +	ret  = ocfs2_new_inode(fs, &tmp_blkno, S_IFDIR | 0755);
> +	if (ret)
> +		FSWRK_COM_FATAL(progname, ret);
> +
> +	ret = ocfs2_init_dir(fs, tmp_blkno, blkno);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +	ret = ocfs2_link(fs, blkno, dirname, tmp_blkno, OCFS2_FT_DIR);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +	*retblkno = tmp_blkno;
> +	
> +}
> +void mess_up_dir_inline_flag(ocfs2_filesys *fs, uint64_t blkno)
> +{
> +	char inline_dirname[OCFS2_MAX_FILENAME_LEN];
> +        char *buf = NULL;
> +        uint64_t inline_blkno;
> +        errcode_t ret;
> +        struct ocfs2_dinode *sb_di = fs->fs_super;
> +        struct ocfs2_dinode *di;
> +
> +        /*mess up inline flag in inode*/
> +        snprintf(inline_dirname,OCFS2_MAX_FILENAME_LEN,
> +                 "inline_dir_mess_flag_%d", getpid());
Are you sure getpid() is ok? normally we corrupt a file with only one 
corruption type, but you use getpid() in mess_up_dir_inline_flag, 
mess_up_dir_inline_size, etc. So you'd better choose another file name.
> +        create_inline_dir(fs,inline_dirname,blkno,&inline_blkno);
> +
> +        ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +        ret = ocfs2_read_inode(fs, inline_blkno, buf);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +        di = (struct ocfs2_dinode *)buf;
> +
> +        if (di->i_dyn_features & OCFS2_INLINE_DATA_FL) {
> +                goto bail;
> +        }
> +        else {
> +                di->i_dyn_features |= OCFS2_INLINE_DATA_FL;
> +                ret = ocfs2_write_inode(fs,inline_blkno,buf);
> +                if (ret)
> +                        FSWRK_COM_FATAL(progname, ret);
> +        }
What do you want to do here? I suppose that you are now trying to 
corrupt a volume which don't have INLINE_DATA support? If the inode 
already has inline-data enabled, you just go out with no corruption.
btw, you don't need to "goto bail" since "bail" is right after your "if".
> +
> +bail:
> +        if (buf)
> +                ocfs2_free(&buf);
> +
> +        return;
> +
> +}
> +void mess_up_dir_inline_size(ocfs2_filesys *fs, uint64_t blkno)
> +{
> +	char inline_dirname[OCFS2_MAX_FILENAME_LEN];
> +        char *buf = NULL;
> +        uint64_t inline_blkno;
> +        errcode_t ret;
> +        struct ocfs2_dinode *di;
> +        uint16_t max_inline_sz;
> +
> +        /*mess up inline flag in inode*/
> +        snprintf(inline_dirname,OCFS2_MAX_FILENAME_LEN,
> +                 "inline_dir_mess_size_%d", getpid());
> +        create_inline_dir(fs,inline_dirname,blkno,&inline_blkno);
> +
> +        ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +        ret = ocfs2_read_inode(fs, inline_blkno, buf);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +        di = (struct ocfs2_dinode *)buf;
> +        max_inline_sz = ocfs2_max_inline_data(fs->fs_blocksize);
> +
> +        if (!(di->i_dyn_features & OCFS2_INLINE_DATA_FL)) {
> +                di->i_dyn_features |= OCFS2_INLINE_DATA_FL;
> +        }
> +        /*mess up id_count,i_size and i_cluster in a inline inode*/
> +        di->id2.i_data.id_count = 0;
> +        di->i_size = max_inline_sz + 1;
> +        di->i_clusters = 1;
> +
> +        ret = ocfs2_write_inode(fs,inline_blkno,buf);
> +        if (ret)
> +                FSWRK_COM_FATAL(progname, ret);
> +
> +        if (buf)
> +                ocfs2_free(&buf);
> +
> +        return;
> +
> +}
> diff --git a/fswreck/include/dir.h b/fswreck/include/dir.h
> index 57afcfb..77e865c 100644
> --- a/fswreck/include/dir.h
> +++ b/fswreck/include/dir.h
> @@ -30,5 +30,7 @@ void mess_up_dir_dot(ocfs2_filesys *fs, uint64_t
> blkno);
>  void mess_up_dir_ent(ocfs2_filesys *fs, uint64_t blkno);
>  void mess_up_dir_parent_dup(ocfs2_filesys *fs, uint64_t blkno);
>  void mess_up_dir_not_connected(ocfs2_filesys *fs, uint64_t blkno);
> +void mess_up_dir_inline_flag(ocfs2_filesys *fs, uint64_t blkno);
> +void mess_up_dir_inline_size(ocfs2_filesys *fs, uint64_t blkno);
>  
>  #endif		/* __DIR_H */
> diff --git a/fswreck/include/fsck_type.h b/fswreck/include/fsck_type.h
> index a6a4873..c443a92 100644
> --- a/fswreck/include/fsck_type.h
> +++ b/fswreck/include/fsck_type.h
> @@ -112,7 +112,15 @@ enum fsck_type
>  	INODE_NOT_CONNECTED,
>  	INODE_COUNT,
>  	INODE_ORPHANED,
> -	NUM_FSCK_TYPE
> +	NUM_FSCK_TYPE,
> +	REGULAR_INLINE_DATA_FLAG,
> +	REGULAR_INLINE_DATA_ID_COUNT,
> +	REGULAR_INLINE_DATA_I_CLUSTER,
> +	REGULAR_INLINE_DATA_I_SIZE,
> +	DIR_INLINE_DATA_FLAG,
> +	DIR_INLINE_DATA_ID_COUNT,
> +	DIR_INLINE_DATA_I_CLUSTER,
> +	DIR_INLINE_DATA_I_SIZE
NUM_FSCK_TYPE, as its name shows, mean the fsck types number, so the 
type should be added before it. And all the fsck types are defined in 
fsck.ocfs2/fsck.ocfs2.checks.8.in, so you should not add others here.

Your modification for normal file corruption is almost the same as dir 
corruption, so it has the same problem.

Regards,
Tao



More information about the Ocfs2-tools-devel mailing list