linux-ext4 - re: ext4: decrement free clusters/inodes counters when block group declared bad

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]

Date:	Mon, 7 Jul 2014 14:44:24 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	namjae.jeon@...sung.com
Cc:	linux-ext4@...r.kernel.org
Subject: re: ext4: decrement free clusters/inodes counters when block group
 declared bad

Hello Namjae Jeon,

This is a semi-automatic email about new static checker warnings.

The patch e43bb4e612b4: "ext4: decrement free clusters/inodes
counters when block group declared bad" from Jun 26, 2014, leads to
the following Smatch complaint:

fs/ext4/ialloc.c:343 ext4_free_inode()
	 error: we previously assumed 'gdp' could be null (see line 300)

fs/ext4/ialloc.c
   299		gdp = ext4_get_group_desc(sb, block_group, &bh2);
   300		if (gdp) {
                    ^^^
Check for NULL.

   301			BUFFER_TRACE(bh2, "get_write_access");
   302			fatal = ext4_journal_get_write_access(handle, bh2);
   303		}
   304		ext4_lock_group(sb, block_group);
   305		cleared = ext4_test_and_clear_bit(bit, bitmap_bh->b_data);
   306		if (fatal || !cleared) {
   307			ext4_unlock_group(sb, block_group);
   308			goto out;
   309		}
   310	
   311		count = ext4_free_inodes_count(sb, gdp) + 1;
   312		ext4_free_inodes_set(sb, gdp, count);
   313		if (is_directory) {
   314			count = ext4_used_dirs_count(sb, gdp) - 1;
   315			ext4_used_dirs_set(sb, gdp, count);
   316			percpu_counter_dec(&sbi->s_dirs_counter);
   317		}
   318		ext4_inode_bitmap_csum_set(sb, block_group, gdp, bitmap_bh,
   319					   EXT4_INODES_PER_GROUP(sb) / 8);
   320		ext4_group_desc_csum_set(sb, block_group, gdp);
   321		ext4_unlock_group(sb, block_group);
   322	
   323		percpu_counter_inc(&sbi->s_freeinodes_counter);
   324		if (sbi->s_log_groups_per_flex) {
   325			ext4_group_t f = ext4_flex_group(sbi, block_group);
   326	
   327			atomic_inc(&sbi->s_flex_groups[f].free_inodes);
   328			if (is_directory)
   329				atomic_dec(&sbi->s_flex_groups[f].used_dirs);
   330		}
   331		BUFFER_TRACE(bh2, "call ext4_handle_dirty_metadata");
   332		fatal = ext4_handle_dirty_metadata(handle, NULL, bh2);
   333	out:
   334		if (cleared) {
   335			BUFFER_TRACE(bitmap_bh, "call ext4_handle_dirty_metadata");
   336			err = ext4_handle_dirty_metadata(handle, NULL, bitmap_bh);
   337			if (!fatal)
   338				fatal = err;
   339		} else {
   340			ext4_error(sb, "bit already cleared for inode %lu", ino);
   341			if (!EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
   342				int count;
   343				count = ext4_free_inodes_count(sb, gdp);
                                                                   ^^^
Patch adds new unchecked dereference inside the function call.

   344				percpu_counter_sub(&sbi->s_freeinodes_counter,
   345						   count);

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