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

Message-ID: <20151103120445.GA20788@mwanda>
Date:	Tue, 3 Nov 2015 15:04:45 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	darrick.wong@...cle.com
Cc:	linux-ext4@...r.kernel.org
Subject: re: ext4: make the bitmap read routines return real error codes

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.

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