[Ocfs2-tools-devel] [PATCH 3/4] Ocfs2-tools: Fix bug for fswreck to handle corruption of inlined directory correctly.

[Joel Becker]

On Fri, Jun 26, 2009 at 10:49:21AM +0800, Tristan Ye wrote:
> +struct dirent_corrupt_struct  {
> +
> +	const char	*oldname;
> +	const char      *name;
> +	int             namelen;
> +	int             done;
> +	int		reserved;
> +};

	Let's add oldnamelen as the length of oldname and
then use namelen as the length of name.  This makes it clear.  Then in
corrupt_match_dirent...

> +static int corrupt_match_dirent(struct dirent_corrupt_struct *dcs,
> +				struct ocfs2_dir_entry *dirent)
>  {
> -	struct ocfs2_dir_entry *de1 = NULL;
> -	uint32_t offset = 0;
> -	uint16_t rec_len = OCFS2_DIR_REC_LEN(namelen);
> -
> -	while (1) {
> -		if ((de->inode == 0 && de->rec_len >= rec_len) ||
> -		    (de->rec_len >= 
> -			(OCFS2_DIR_REC_LEN(de->name_len) + rec_len))) {
> -			offset += de->rec_len;
> -			if (de->inode) {
> -			de1 = (struct ocfs2_dir_entry *)((char *) de +
> -				OCFS2_DIR_REC_LEN(de->name_len));
> -			de1->rec_len = de->rec_len -
> -				OCFS2_DIR_REC_LEN(de->name_len);
> -			de->rec_len = OCFS2_DIR_REC_LEN(de->name_len);
> -			de = de1;
> -			}
> -			de->file_type = OCFS2_FT_UNKNOWN;
> -			de->inode = blkno;
> -			ocfs2_set_de_type(de, mode);
> -			de->name_len = namelen;
> -			memcpy(de->name, name, namelen);
> -			break;
> -		}
> -		offset += de->rec_len;
> -		de = (struct ocfs2_dir_entry *) ((char *) de + de->rec_len);
> -		if (offset >= fs->fs_blocksize)
> -			FSWRK_FATAL("no space for the new dirent");
> -	}
> +	if (!dcs->oldname)
> +		return 1;
>  
> -	*retent = de;
> -	return;
> +	if (((dirent->name_len & 0xFF) != dcs->namelen))
> +		return 0;

	We compare dcs->oldnamelen here.

> +static int rename_dirent_proc(struct ocfs2_dir_entry *dirent,
> +			      int        offset,
> +			      int        blocksize,
> +			      char       *buf,
> +			      void       *priv_data)
>  {
> -	errcode_t ret;
> -	char *buf = NULL;
> -	uint64_t blkno, tmp_blkno;
> -	uint64_t contig;
> -	ocfs2_cached_inode *cinode = NULL;
> -	struct ocfs2_dir_entry *de = NULL, *newent = NULL;
> -	char name[OCFS2_MAX_FILENAME_LEN];
> -	int namelen;
> -	mode_t mode;
> +	struct  dirent_corrupt_struct *dcs = (struct dirent_corrupt_struct *) priv_data;
>  
> -	ret = ocfs2_read_cached_inode(fs, dir, &cinode);
> -	if (ret)
> -		FSWRK_COM_FATAL(progname, ret);
> +	if (!corrupt_match_dirent(dcs, dirent))
> +		return 0;
> +	
> +	strcpy(dirent->name, dcs->name);

	Here you store dcs->name, but what if dcs->name is not the same
length as dirent->name?  Currently fswreck only does a corruption of
'.' -> 'a', so the length is the same.  But the limitation is not
obvious.  You can do one of two things:

1) The easy thing is to make the limitation explicit.  At the top of
rename_dirent() you check that dcs->oldnamelen == dcs->namelen or you
FSWRECK_FATAL.

2) The fancier thing would be to check here if dcs->namelen fits inside
the dirent (that is, dcs->namelen <= (dirent->rec_len - offsetof(struct
ocfs2_dir_entry, name))).  If it fits, you store the new name and new
namelen.  If it does not, you FATAL.

Joel

-- 

Life's Little Instruction Book #274

	"Leave everything a little better than you found it."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127