ext4: potential NULL dereference on error

The ext4_get_group_desc() function returns NULL on error, and
ext4_free_inodes_count() function dereferences it without checking.
There is a check on the next line, but it's too late.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static checker fix.

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

On Sun 13-05-12 17:41:04, Dan Carpenter wrote:
> The ext4_get_group_desc() function returns NULL on error, and
> ext4_free_inodes_count() function dereferences it without checking.
> There is a check on the next line, but it's too late.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static checker fix.
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index a044a9b..1526f33 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -389,7 +389,7 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	ext4_group_t real_ngroups = ext4_get_groups_count(sb);
>  	int inodes_per_group = EXT4_INODES_PER_GROUP(sb);
> -	unsigned int freei, avefreei, grp_free;
> +	unsigned int freei, avefreei;
>  	ext4_fsblk_t freeb, avefreec;
>  	unsigned int ndirs;
>  	int max_dirs, min_inodes;
> @@ -399,6 +399,7 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent,
>  	struct orlov_stats stats;
>  	int flex_size = ext4_flex_bg_size(sbi);
>  	struct dx_hash_info hinfo;
> +	unsigned int grp_free = 0;
>  
>  	ngroups = real_ngroups;
>  	if (flex_size > 1) {
> @@ -508,7 +509,8 @@ fallback_retry:
>  	for (i = 0; i < ngroups; i++) {
>  		grp = (parent_group + i) % ngroups;
>  		desc = ext4_get_group_desc(sb, grp, NULL);
> -		grp_free = ext4_free_inodes_count(sb, desc);
> +		if (desc)
> +			grp_free = ext4_free_inodes_count(sb, desc);
>  		if (desc && grp_free && grp_free >= avefreei) {
  So you it would be more logical to do:
		if (desc) {
			grp_free = ext4_free_inodes_count(sb, desc);
			if (grp_free && grpfree >= avefreei) {
	 			*group = grp;
	 			return 0;
			}
		}

  Wouldn't it?

								Honza

On Tue, May 15, 2012 at 12:25:35AM +0200, Jan Kara wrote:
> On Sun 13-05-12 17:41:04, Dan Carpenter wrote:
> > @@ -508,7 +509,8 @@ fallback_retry:
> >  	for (i = 0; i < ngroups; i++) {
> >  		grp = (parent_group + i) % ngroups;
> >  		desc = ext4_get_group_desc(sb, grp, NULL);
> > -		grp_free = ext4_free_inodes_count(sb, desc);
> > +		if (desc)
> > +			grp_free = ext4_free_inodes_count(sb, desc);
> >  		if (desc && grp_free && grp_free >= avefreei) {
>   So you it would be more logical to do:
> 		if (desc) {
> 			grp_free = ext4_free_inodes_count(sb, desc);
> 			if (grp_free && grpfree >= avefreei) {
> 	 			*group = grp;
> 	 			return 0;
> 			}
> 		}
> 
>   Wouldn't it?

Yeah.  You're obviously right.  I'll resend.

regards,
dan carpenter

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