[PATCH 0/3] Misc fixes silencing false positives from cppcheck

[PATCH 0/3] Misc fixes silencing false positives from cppcheck

[Johannes Thumshirn]

misc fixes provoked by https://bugzilla.kernel.org/show_bug.cgi?id=205003

The first patch removes an unneeded variable, which when removed silences
a false positive warning generated by cppcheck.

The second patch removes a BUG_ON() which can't be hit, for the same
reasons as there can't be no NULL-pointer dereference in patch 1.

Patch number three removes a WARN_ON() which can't be triggered. This
WARN_ON() was also identified as a possible NULL-pointer dereference by
cppcheck.


Johannes Thumshirn (3):
  btrfs: fix possible NULL-pointer dereference in integrity checks
  btrfs: remove superfluous BUG_ON() in integrity checks
  btrfs: remove impossible WARN_ON in btrfs_destroy_dev_replace_tgtdev()

 fs/btrfs/check-integrity.c | 4 +---
 fs/btrfs/volumes.c         | 1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

-- 
2.21.0 (Apple Git-122)

[PATCH 1/3] btrfs: fix possible NULL-pointer dereference in integrity checks

[Johannes Thumshirn]

A user reports a possible NULL-pointer dereference in
btrfsic_process_superblock(). We are assigning state->fs_info to a local
fs_info variable and afterwards checking for the presence of state.

While we would BUG_ON() a NULL state anyways, we can also just remove the
local fs_info copy, as fs_info is only used once as the first argument for
btrfs_num_copies(). There we can just pass in state->fs_info as well.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=205003
Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
 fs/btrfs/check-integrity.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 0b52ab4cb964..72c70f59fc60 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -629,7 +629,6 @@ static struct btrfsic_dev_state *btrfsic_dev_state_hashtable_lookup(dev_t dev,
 static int btrfsic_process_superblock(struct btrfsic_state *state,
 				      struct btrfs_fs_devices *fs_devices)
 {
-	struct btrfs_fs_info *fs_info = state->fs_info;
 	struct btrfs_super_block *selected_super;
 	struct list_head *dev_head = &fs_devices->devices;
 	struct btrfs_device *device;
@@ -700,7 +699,7 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 			break;
 		}
 
-		num_copies = btrfs_num_copies(fs_info, next_bytenr,
+		num_copies = btrfs_num_copies(state->fs_info, next_bytenr,
 					      state->metablock_size);
 		if (state->print_mask & BTRFSIC_PRINT_MASK_NUM_COPIES)
 			pr_info("num_copies(log_bytenr=%llu) = %d\n",
-- 
2.21.0 (Apple Git-122)

[PATCH 2/3] btrfs: remove superfluous BUG_ON() in integrity checks

[Johannes Thumshirn]

btrfsic_process_superblock() BUG_ON()s if 'state' is NULL. But this can
never happen as the only caller from btrfsic_process_superblock() is btrfsic_mount()
which allocates 'state' some lines above calling
btrfsic_process_superblock() and checks for the allocation to succeed.

Let's just remove the impossible to hit BUG_ON().

Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
 fs/btrfs/check-integrity.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 72c70f59fc60..a0ce69f2d27c 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -636,7 +636,6 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 	int ret = 0;
 	int pass;
 
-	BUG_ON(NULL == state);
 	selected_super = kzalloc(sizeof(*selected_super), GFP_NOFS);
 	if (NULL == selected_super) {
 		pr_info("btrfsic: error, kmalloc failed!\n");
-- 
2.21.0 (Apple Git-122)

[PATCH 3/3] btrfs: remove impossible WARN_ON in btrfs_destroy_dev_replace_tgtdev()

[Johannes Thumshirn]

We have a user report, that cppcheck is complaining about a possible
NULL-pointer dereference in btrfs_destroy_dev_replace_tgtdev().

We're first dereferencing the 'tgtdev' variable and the later check for
the validity of the pointer with a WARN_ON(!tgtdev);

But all callers of btrfs_destroy_dev_replace_tgtdev() either explicitly
check if 'tgtdev' is non-NULL or directly allocate 'tgtdev', so the
WARN_ON() is impossible to hit. Just remove it to silence the checker's
complains.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=205003
Signed-off-by: Johannes Thumshirn <jth@kernel.org>
---
 fs/btrfs/volumes.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index c565650639ee..f5c0c401c330 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2132,7 +2132,6 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev)
 {
 	struct btrfs_fs_devices *fs_devices = tgtdev->fs_info->fs_devices;
 
-	WARN_ON(!tgtdev);
 	mutex_lock(&fs_devices->device_list_mutex);
 
 	btrfs_sysfs_rm_device_link(fs_devices, tgtdev);
-- 
2.21.0 (Apple Git-122)

Re: [PATCH 0/3] Misc fixes silencing false positives from cppcheck

[David Sterba]

On Thu, Dec 05, 2019 at 02:19:56PM +0100, Johannes Thumshirn wrote:
> misc fixes provoked by https://bugzilla.kernel.org/show_bug.cgi?id=205003
> 
> The first patch removes an unneeded variable, which when removed silences
> a false positive warning generated by cppcheck.
> 
> The second patch removes a BUG_ON() which can't be hit, for the same
> reasons as there can't be no NULL-pointer dereference in patch 1.
> 
> Patch number three removes a WARN_ON() which can't be triggered. This
> WARN_ON() was also identified as a possible NULL-pointer dereference by
> cppcheck.
> 
> 
> Johannes Thumshirn (3):
>   btrfs: fix possible NULL-pointer dereference in integrity checks
>   btrfs: remove superfluous BUG_ON() in integrity checks
>   btrfs: remove impossible WARN_ON in btrfs_destroy_dev_replace_tgtdev()

Added to misc-next, thanks.