[PATCH 0/2] btrfs: Move dereference behind null checks

[PATCH 0/2] btrfs: Move dereference behind null checks

[Sebastian]

From: Sebastian Scherbel <sebastian.scherbel@fau.de>

Regarding Bug 205003, points 1 and 2
This patch series moves two dereferences after the null check to avoid
a possible null pointer dereference.

Sebastian Scherbel (2):
  btrfs: Move dereference behind null check in check integrity
  btrfs: Move dereference behind null check in check volumes

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

-- 
2.20.1

[PATCH 1/2] btrfs: Move dereference behind null check in check integrity

[Sebastian]

From: Sebastian Scherbel <sebastian.scherbel@fau.de>

Regarding Bug 205003, point 1
The struct "state" is currently dereferenced before being checked
for null later on. This patch moves the dereference after the null check
to avoid a possible null pointer dereference.

Signed-off-by: Sebastian Scherbel <sebastian.scherbel@fau.de>
Co-developed-by: Ole Wiedemann <ole.wiedemann@fau.de>
Signed-off-by: Ole Wiedemann <ole.wiedemann@fau.de>
---
 fs/btrfs/check-integrity.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 0b52ab4cb964..fc429436765c 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -629,7 +629,7 @@ 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_fs_info *fs_info;
 	struct btrfs_super_block *selected_super;
 	struct list_head *dev_head = &fs_devices->devices;
 	struct btrfs_device *device;
@@ -638,6 +638,8 @@ static int btrfsic_process_superblock(struct btrfsic_state *state,
 	int pass;
 
 	BUG_ON(NULL == state);
+	fs_info = state->fs_info;
+
 	selected_super = kzalloc(sizeof(*selected_super), GFP_NOFS);
 	if (NULL == selected_super) {
 		pr_info("btrfsic: error, kmalloc failed!\n");
-- 
2.20.1

[PATCH 2/2] btrfs: Move dereference behind null check in check volumes

[Sebastian]

From: Sebastian Scherbel <sebastian.scherbel@fau.de>

Regarding Bug 205003, point 2
The struct "tgtdev" is currently dereferenced before being checked
for null later on. This patch moves the derefernce after the null
check to avoid a possible null pointer dereference.
Furthermore WARN_ON is replaced by BUG_ON to prevent the subsequent
dereference of the null pointer.

Signed-off-by: Sebastian Scherbel <sebastian.scherbel@fau.de>
Co-developed-by: Ole Wiedemann <ole.wiedemann@fau.de>
Signed-off-by: Ole Wiedemann <ole.wiedemann@fau.de>
---
 fs/btrfs/volumes.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d8e5560db285..12015f60f50d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2128,9 +2128,11 @@ void btrfs_rm_dev_replace_free_srcdev(struct btrfs_device *srcdev)
 
 void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev)
 {
-	struct btrfs_fs_devices *fs_devices = tgtdev->fs_info->fs_devices;
+	struct btrfs_fs_devices *fs_devices;
+
+	BUG_ON(!tgtdev);
+	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.20.1

Re: [PATCH 0/2] btrfs: Move dereference behind null checks

[Johannes Thumshirn]

On 07/12/2019 23:18, Sebastian wrote:
> From: Sebastian Scherbel <sebastian.scherbel@fau.de>
> 
> Regarding Bug 205003, points 1 and 2
> This patch series moves two dereferences after the null check to avoid
> a possible null pointer dereference.
> 
> Sebastian Scherbel (2):
>   btrfs: Move dereference behind null check in check integrity
>   btrfs: Move dereference behind null check in check volumes
> 
>  fs/btrfs/check-integrity.c | 4 +++-
>  fs/btrfs/volumes.c         | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 


Hi,

I've already submitted a series addressing these and David merged it:
https://lore.kernel.org/linux-btrfs/20191205131959.19184-1-jth@kernel.org/

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 0/2] btrfs: Move dereference behind null checks

[David Sterba]

On Mon, Dec 09, 2019 at 10:30:06AM +0100, Johannes Thumshirn wrote:
> On 07/12/2019 23:18, Sebastian wrote:
> > From: Sebastian Scherbel <sebastian.scherbel@fau.de>
> > 
> > Regarding Bug 205003, points 1 and 2
> > This patch series moves two dereferences after the null check to avoid
> > a possible null pointer dereference.
> > 
> > Sebastian Scherbel (2):
> >   btrfs: Move dereference behind null check in check integrity
> >   btrfs: Move dereference behind null check in check volumes
> > 
> >  fs/btrfs/check-integrity.c | 4 +++-
> >  fs/btrfs/volumes.c         | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> 
> I've already submitted a series addressing these and David merged it:
> https://lore.kernel.org/linux-btrfs/20191205131959.19184-1-jth@kernel.org/

Yes, that's been in misc-next since ~friday, I'm not sure when I pushed
the branch.

Sebastian, thanks for the patches, this sometimes happens that the work
is duplicated. Johannes removed the BUG_ON and WARN_ON completely though
your change is also correct assuming that the two are not redundant.