[cocci] [PATCH resent 1/2] md/raid1: Fix exception handling in setup_conf()

[Markus Elfring <Markus.Elfring@web.de>]

Date: Mon, 20 Mar 2023 16:40:11 +0100

The label “abort” was used to jump to another pointer check despite of
the detail in the implementation of the function “setup_conf”
that it was determined already that a corresponding variable contained
still a null pointer because of a failed function call.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.

4. Move three assignments for the variable “err”.


This issue was detected by using the Coccinelle software.

Fixes: 709ae4879ae33628ded276ce7da8cd5acfec476b ("md/raid1: add takeover support for raid5->raid1")
Fixes: fd76863e37fef26fe05547fddfa6e3d05e1682e6 ("RAID1: a new I/O barrier implementation to remove resync window")
Fixes: afeee514ce7f4cab605beedd03be71ebaf0c5fc8 ("md: convert to bioset_init()/mempool_init()")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid1.c | 75 +++++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 68a9e2d9985b..b614f50505ec 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2978,54 +2978,54 @@ static struct r1conf *setup_conf(struct mddev *mddev)

 	conf = kzalloc(sizeof(struct r1conf), GFP_KERNEL);
 	if (!conf)
-		goto abort;
+		goto exit;

 	conf->nr_pending = kcalloc(BARRIER_BUCKETS_NR,
 				   sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_pending)
-		goto abort;
+		goto free_conf;

 	conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
 				   sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_waiting)
-		goto abort;
+		goto free_pending;

 	conf->nr_queued = kcalloc(BARRIER_BUCKETS_NR,
 				  sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_queued)
-		goto abort;
+		goto free_waiting;

 	conf->barrier = kcalloc(BARRIER_BUCKETS_NR,
 				sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->barrier)
-		goto abort;
+		goto free_queued;

 	conf->mirrors = kzalloc(array3_size(sizeof(struct raid1_info),
 					    mddev->raid_disks, 2),
 				GFP_KERNEL);
 	if (!conf->mirrors)
-		goto abort;
+		goto free_barrier;

 	conf->tmppage = alloc_page(GFP_KERNEL);
 	if (!conf->tmppage)
-		goto abort;
+		goto free_mirrors;

 	conf->poolinfo = kzalloc(sizeof(*conf->poolinfo), GFP_KERNEL);
 	if (!conf->poolinfo)
-		goto abort;
+		goto put_page;
+
 	conf->poolinfo->raid_disks = mddev->raid_disks * 2;
 	err = mempool_init(&conf->r1bio_pool, NR_RAID_BIOS, r1bio_pool_alloc,
 			   rbio_pool_free, conf->poolinfo);
 	if (err)
-		goto abort;
+		goto free_poolinfo;

 	err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (err)
-		goto abort;
+		goto mempool_exit;

 	conf->poolinfo->mddev = mddev;

-	err = -EINVAL;
 	spin_lock_init(&conf->device_lock);
 	rdev_for_each(rdev, mddev) {
 		int disk_idx = rdev->raid_disk;
@@ -3037,8 +3037,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		else
 			disk = conf->mirrors + disk_idx;

-		if (disk->rdev)
-			goto abort;
+		if (disk->rdev) {
+			err = -EINVAL;
+			goto bioset_exit;
+		}
+
 		disk->rdev = rdev;
 		disk->head_position = 0;
 		disk->seq_start = MaxSector;
@@ -3054,7 +3057,6 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 	bio_list_init(&conf->pending_bio_list);
 	conf->recovery_disabled = mddev->recovery_disabled - 1;

-	err = -EIO;
 	for (i = 0; i < conf->raid_disks * 2; i++) {

 		disk = conf->mirrors + i;
@@ -3069,9 +3071,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 				disk->rdev =
 					disk[conf->raid_disks].rdev;
 				disk[conf->raid_disks].rdev = NULL;
-			} else if (!test_bit(In_sync, &disk->rdev->flags))
+			} else if (!test_bit(In_sync, &disk->rdev->flags)) {
 				/* Original is not in_sync - bad */
-				goto abort;
+				err = -EIO;
+				goto bioset_exit;
+			}
 		}

 		if (!disk->rdev ||
@@ -3083,26 +3087,35 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		}
 	}

-	err = -ENOMEM;
 	conf->thread = md_register_thread(raid1d, mddev, "raid1");
 	if (!conf->thread)
-		goto abort;
+		goto e_nomem;

 	return conf;

- abort:
-	if (conf) {
-		mempool_exit(&conf->r1bio_pool);
-		kfree(conf->mirrors);
-		safe_put_page(conf->tmppage);
-		kfree(conf->poolinfo);
-		kfree(conf->nr_pending);
-		kfree(conf->nr_waiting);
-		kfree(conf->nr_queued);
-		kfree(conf->barrier);
-		bioset_exit(&conf->bio_split);
-		kfree(conf);
-	}
+e_nomem:
+	err = -ENOMEM;
+bioset_exit:
+	bioset_exit(&conf->bio_split);
+mempool_exit:
+	mempool_exit(&conf->r1bio_pool);
+free_poolinfo:
+	kfree(conf->poolinfo);
+put_page:
+	safe_put_page(conf->tmppage);
+free_mirrors:
+	kfree(conf->mirrors);
+free_barrier:
+	kfree(conf->barrier);
+free_queued:
+	kfree(conf->nr_queued);
+free_waiting:
+	kfree(conf->nr_waiting);
+free_pending:
+	kfree(conf->nr_pending);
+free_conf:
+	kfree(conf);
+exit:
 	return ERR_PTR(err);
 }

--
2.40.0