linux-ext4 - Re: ext4: make the bitmap read routines return real error codes

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: <20151103192041.GC2223@birch.djwong.org>
Date:	Tue, 3 Nov 2015 11:20:41 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: ext4: make the bitmap read routines return real error codes

On Tue, Nov 03, 2015 at 03:04:45PM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
> 
> The patch 7d6232775976: "ext4: make the bitmap read routines return
> real error codes" from Oct 17, 2015, leads to the following static
> checker warning:
> 
> 	fs/ext4/mballoc.c:2989 ext4_mb_mark_diskspace_used()
> 	error: 'bitmap_bh' dereferencing possible ERR_PTR()
> 
> fs/ext4/mballoc.c
>   2899          bitmap_bh = ext4_read_block_bitmap(sb, ac->ac_b_ex.fe_group);
>   2900          if (IS_ERR(bitmap_bh)) {
>   2901                  err = PTR_ERR(bitmap_bh);
>   2902                  goto out_err;
>   2903          }
>   2904  
> 
> [ snip ]
> 
>   2987  
>   2988  out_err:
>   2989          brelse(bitmap_bh);
>   2990          return err;
>   2991  }
> 
> Also:
> 
> fs/ext4/mballoc.c:4894 ext4_free_blocks() error: 'bitmap_bh' dereferencing possible ERR_PTR()
> fs/ext4/mballoc.c:5028 ext4_group_add_blocks() error: 'bitmap_bh' dereferencing possible ERR_PTR()
> 
> This is One Err style error handling where one error label handles every
> possible error so it's error prone (handling every error is more
> complicated than doing a specific thing).
> 
> The old code relied on the sanity check in brelse() to avoid NULL
> dereferences but now we are passing ERR_PTRs so it's not enough.
> Probably the fix is to update the sanity check in brelse().  Another
> idea would be to not free things until they have been allocated.

Or just slip in a "bitmap_bh = NULL;" just before line 2902.  We've saved
the error code, so the pointer can be zeroed.

Hmm, thank you for the report, I'll get a patch out soon.  Guess I should
go figure out how to smatch-scan my dev tree. :)

(Particularly because I fixed this exact problem in other parts of the patch,
but not here. Sigh.)

--D

> 
> regards,
> dan carpenter
--
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