[PATCH V4 00/14] mdadm: fix coverity issues

[PATCH V4 00/14] mdadm: fix coverity issues

[Xiao Ni]

V2: replace close with close_fd and use is_fd_valid
V3: Fix errors reported by checkpatch
V4:
don't need check fd is valid before close_fd
replace strcat with snprintf
replace dev_name with dev_path_name

Xiao Ni (14):
  mdadm/Grow: fix coverity issue CHECKED_RETURN
  mdadm/Grow: fix coverity issue RESOURCE_LEAK
  mdadm/Grow: fix coverity issue STRING_OVERFLOW
  mdadm/Incremental: fix coverity issues.
  mdadm/mdmon: fix coverity issue CHECKED_RETURN
  mdadm/mdmon: fix coverity issue RESOURCE_LEAK
  mdadm/mdopen: fix coverity issue CHECKED_RETURN
  mdadm/mdopen: fix coverity issue STRING_OVERFLOW
  mdadm/mdstat: fix coverity issue CHECKED_RETURN
  mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER
  mdadm/super1: fix coverity issue CHECKED_RETURN
  mdadm/super1: fix coverity issue DEADCODE
  mdadm/super1: fix coverity issue EVALUATION_ORDER
  mdadm/super1: fix coverity issue RESOURCE_LEAK

 Grow.c        | 87 ++++++++++++++++++++++++++++++++++++++++-----------
 Incremental.c | 20 ++++++------
 mdmon.c       | 20 +++++++++---
 mdopen.c      |  8 +++--
 mdstat.c      | 12 +++++--
 super0.c      | 10 ++++--
 super1.c      | 32 ++++++++++++++-----
 7 files changed, 139 insertions(+), 50 deletions(-)

-- 
2.32.0 (Apple Git-132)

[PATCH 01/14] mdadm/Grow: fix coverity issue CHECKED_RETURN

[Xiao Ni]

It needs to check return value when functions have return value.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Grow.c | 43 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/Grow.c b/Grow.c
index b135930d05b8..7ae967bda067 100644
--- a/Grow.c
+++ b/Grow.c
@@ -3261,7 +3261,12 @@ static int reshape_array(char *container, int fd, char *devname,
 					/* This is a spare that wants to
 					 * be part of the array.
 					 */
-					add_disk(fd, st, info2, d);
+					if (add_disk(fd, st, info2, d) < 0) {
+						pr_err("Can not add disk %s\n",
+								d->sys_name);
+						free(info2);
+						goto release;
+					}
 				}
 			}
 			sysfs_free(info2);
@@ -4413,7 +4418,10 @@ static void validate(int afd, int bfd, unsigned long long offset)
 	 */
 	if (afd < 0)
 		return;
-	lseek64(bfd, offset - 4096, 0);
+	if (lseek64(bfd, offset - 4096, 0) < 0) {
+		pr_err("lseek64 fails %d:%s\n", errno, strerror(errno));
+		return;
+	}
 	if (read(bfd, &bsb2, 512) != 512)
 		fail("cannot read bsb");
 	if (bsb2.sb_csum != bsb_csum((char*)&bsb2,
@@ -4444,12 +4452,19 @@ static void validate(int afd, int bfd, unsigned long long offset)
 			}
 		}
 
-		lseek64(bfd, offset, 0);
+		if (lseek64(bfd, offset, 0) < 0) {
+			pr_err("lseek64 fails %d:%s\n", errno, strerror(errno));
+			goto out;
+		}
 		if ((unsigned long long)read(bfd, bbuf, len) != len) {
 			//printf("len %llu\n", len);
 			fail("read first backup failed");
 		}
-		lseek64(afd, __le64_to_cpu(bsb2.arraystart)*512, 0);
+
+		if (lseek64(afd, __le64_to_cpu(bsb2.arraystart)*512, 0) < 0) {
+			pr_err("lseek64 fails %d:%s\n", errno, strerror(errno));
+			goto out;
+		}
 		if ((unsigned long long)read(afd, abuf, len) != len)
 			fail("read first from array failed");
 		if (memcmp(bbuf, abuf, len) != 0)
@@ -4466,15 +4481,25 @@ static void validate(int afd, int bfd, unsigned long long offset)
 			bbuf = xmalloc(abuflen);
 		}
 
-		lseek64(bfd, offset+__le64_to_cpu(bsb2.devstart2)*512, 0);
+		if (lseek64(bfd, offset+__le64_to_cpu(bsb2.devstart2)*512, 0) < 0) {
+			pr_err("lseek64 fails %d:%s\n", errno, strerror(errno));
+			goto out;
+		}
 		if ((unsigned long long)read(bfd, bbuf, len) != len)
 			fail("read second backup failed");
-		lseek64(afd, __le64_to_cpu(bsb2.arraystart2)*512, 0);
+		if (lseek64(afd, __le64_to_cpu(bsb2.arraystart2)*512, 0) < 0) {
+			pr_err("lseek64 fails %d:%s\n", errno, strerror(errno));
+			goto out;
+		}
 		if ((unsigned long long)read(afd, abuf, len) != len)
 			fail("read second from array failed");
 		if (memcmp(bbuf, abuf, len) != 0)
 			fail("data2 compare failed");
 	}
+out:
+	free(abuf);
+	free(bbuf);
+	return;
 }
 
 int child_monitor(int afd, struct mdinfo *sra, struct reshape *reshape,
@@ -5033,7 +5058,11 @@ int Grow_continue_command(char *devname, int fd, struct context *c)
 			goto Grow_continue_command_exit;
 		}
 		content = &array;
-		sysfs_init(content, fd, NULL);
+		if (sysfs_init(content, fd, NULL) < 0) {
+			pr_err("sysfs_init fails\n");
+			ret_val = 1;
+			goto Grow_continue_command_exit;
+		}
 		/* Need to load a superblock.
 		 * FIXME we should really get what we need from
 		 * sysfs
-- 
2.32.0 (Apple Git-132)

[PATCH 02/14] mdadm/Grow: fix coverity issue RESOURCE_LEAK

[Xiao Ni]

Fix some resource leak problems.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Grow.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/Grow.c b/Grow.c
index 7ae967bda067..907a6e1b9e22 100644
--- a/Grow.c
+++ b/Grow.c
@@ -530,8 +530,10 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
 				pr_err("Cannot add bitmap while array is resyncing or reshaping etc.\n");
 			pr_err("Cannot set bitmap file for %s: %s\n",
 				devname, strerror(err));
+			close_fd(&bitmap_fd);
 			return 1;
 		}
+		close_fd(&bitmap_fd);
 	}
 
 	return 0;
@@ -3083,6 +3085,7 @@ static int reshape_array(char *container, int fd, char *devname,
 	int done;
 	struct mdinfo *sra = NULL;
 	char buf[SYSFS_MAX_BUF_SIZE];
+	bool located_backup = false;
 
 	/* when reshaping a RAID0, the component_size might be zero.
 	 * So try to fix that up.
@@ -3165,8 +3168,10 @@ static int reshape_array(char *container, int fd, char *devname,
 			goto release;
 		}
 
-		if (!backup_file)
+		if (!backup_file) {
 			backup_file = locate_backup(sra->sys_name);
+			located_backup = true;
+		}
 
 		goto started;
 	}
@@ -3612,15 +3617,13 @@ started:
 			mdstat_wait(30 - (delayed-1) * 25);
 	} while (delayed);
 	mdstat_close();
-	if (check_env("MDADM_GROW_VERIFY"))
-		fd = open(devname, O_RDONLY | O_DIRECT);
-	else
-		fd = -1;
 	mlockall(MCL_FUTURE);
 
 	if (signal_s(SIGTERM, catch_term) == SIG_ERR)
 		goto release;
 
+	if (check_env("MDADM_GROW_VERIFY"))
+		fd = open(devname, O_RDONLY | O_DIRECT);
 	if (st->ss->external) {
 		/* metadata handler takes it from here */
 		done = st->ss->manage_reshape(
@@ -3632,6 +3635,7 @@ started:
 			fd, sra, &reshape, st, blocks, fdlist, offsets,
 			d - odisks, fdlist + odisks, offsets + odisks);
 
+	close_fd(&fd);
 	free(fdlist);
 	free(offsets);
 
@@ -3701,6 +3705,8 @@ out:
 	exit(0);
 
 release:
+	if (located_backup)
+		free(backup_file);
 	free(fdlist);
 	free(offsets);
 	if (orig_level != UnSet && sra) {
@@ -3839,6 +3845,7 @@ int reshape_container(char *container, char *devname,
 			pr_err("Unable to initialize sysfs for %s\n",
 			       mdstat->devnm);
 			rv = 1;
+			close_fd(&fd);
 			break;
 		}
 
@@ -4717,6 +4724,7 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 	unsigned long long *offsets;
 	unsigned long long  nstripe, ostripe;
 	int ndata, odata;
+	int fd, backup_fd = -1;
 
 	odata = info->array.raid_disks - info->delta_disks - 1;
 	if (info->array.level == 6)
@@ -4732,9 +4740,18 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 		 * been used
 		 */
 		old_disks = cnt;
+
+	if (backup_file) {
+		backup_fd = open(backup_file, O_RDONLY);
+		if (!is_fd_valid(backup_fd)) {
+			pr_err("Can't open backup file %s : %s\n",
+				backup_file, strerror(errno));
+			return -EINVAL;
+		}
+	}
+
 	for (i=old_disks-(backup_file?1:0); i<cnt; i++) {
 		struct mdinfo dinfo;
-		int fd;
 		int bsbsize;
 		char *devname, namebuf[20];
 		unsigned long long lo, hi;
@@ -4747,12 +4764,9 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 		 * else restore data and update all superblocks
 		 */
 		if (i == old_disks-1) {
-			fd = open(backup_file, O_RDONLY);
-			if (fd<0) {
-				pr_err("backup file %s inaccessible: %s\n",
-					backup_file, strerror(errno));
+			if (!is_fd_valid(backup_fd))
 				continue;
-			}
+			fd = backup_fd;
 			devname = backup_file;
 		} else {
 			fd = fdlist[i];
@@ -4907,6 +4921,7 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 				pr_err("Error restoring backup from %s\n",
 					devname);
 			free(offsets);
+			close_fd(&backup_fd);
 			return 1;
 		}
 
@@ -4923,6 +4938,7 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 				pr_err("Error restoring second backup from %s\n",
 					devname);
 			free(offsets);
+			close_fd(&backup_fd);
 			return 1;
 		}
 
@@ -4984,8 +5000,12 @@ int Grow_restart(struct supertype *st, struct mdinfo *info, int *fdlist,
 			st->ss->store_super(st, fdlist[j]);
 			st->ss->free_super(st);
 		}
+		close_fd(&backup_fd);
 		return 0;
 	}
+
+	close_fd(&backup_fd);
+
 	/* Didn't find any backup data, try to see if any
 	 * was needed.
 	 */
-- 
2.32.0 (Apple Git-132)

[PATCH 03/14] mdadm/Grow: fix coverity issue STRING_OVERFLOW

[Xiao Ni]

Fix string overflow problems in Grow.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Grow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Grow.c b/Grow.c
index 907a6e1b9e22..a5f9027d93d8 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1694,7 +1694,7 @@ char *analyse_change(char *devname, struct mdinfo *info, struct reshape *re)
 					/* Current RAID6 layout has a RAID5
 					 * equivalent - good
 					 */
-					strcat(strcpy(layout, ls), "-6");
+					snprintf(layout, 40, "%s-6", ls);
 					l = map_name(r6layout, layout);
 					if (l == UnSet)
 						return "Cannot find RAID6 layout to convert to";
-- 
2.32.0 (Apple Git-132)

[PATCH 04/14] mdadm/Incremental: fix coverity issues.

[Xiao Ni]

There are two issues PW.PARAMETER_HIDDEN (declaration hides
parameter 'devname') and INTEGER_OVERFLOW.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Incremental.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index 83db071214ee..33e6814de9d0 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -770,7 +770,7 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
 			replcnt++;
 		st->ss->free_super(st);
 	}
-	if (max_journal_events >= max_events - 1)
+	if (max_events > 0 && max_journal_events >= max_events - 1)
 		bestinfo->journal_clean = 1;
 
 	if (!avail)
@@ -1113,7 +1113,7 @@ static int partition_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		int fd = -1;
 		struct mdinfo info;
 		struct supertype *st2 = NULL;
-		char *devname = NULL;
+		char *dev_path_name = NULL;
 		unsigned long long devsectors;
 		char *pathlist[2];
 
@@ -1142,14 +1142,14 @@ static int partition_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		domain_free(domlist);
 		domlist = NULL;
 
-		if (asprintf(&devname, "/dev/disk/by-path/%s", de->d_name) != 1) {
-			devname = NULL;
+		if (asprintf(&dev_path_name, "/dev/disk/by-path/%s", de->d_name) != 1) {
+			dev_path_name = NULL;
 			goto next;
 		}
-		fd = open(devname, O_RDONLY);
+		fd = open(dev_path_name, O_RDONLY);
 		if (fd < 0)
 			goto next;
-		if (get_dev_size(fd, devname, &devsectors) == 0)
+		if (get_dev_size(fd, dev_path_name, &devsectors) == 0)
 			goto next;
 		devsectors >>= 9;
 
@@ -1188,8 +1188,8 @@ static int partition_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		if (chosen == NULL || chosen_size < info.component_size) {
 			chosen_size = info.component_size;
 			free(chosen);
-			chosen = devname;
-			devname = NULL;
+			chosen = dev_path_name;
+			dev_path_name = NULL;
 			if (chosen_st) {
 				chosen_st->ss->free_super(chosen_st);
 				free(chosen_st);
@@ -1199,7 +1199,7 @@ static int partition_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
 		}
 
 	next:
-		free(devname);
+		free(dev_path_name);
 		domain_free(domlist);
 		dev_policy_free(pol2);
 		if (st2)
@@ -1246,7 +1246,7 @@ static int is_bare(int dfd)
 
 	/* OK, first 4K appear blank, try the end. */
 	get_dev_size(dfd, NULL, &size);
-	if (lseek(dfd, size-4096, SEEK_SET) < 0 ||
+	if ((size >= 4096 && lseek(dfd, size-4096, SEEK_SET) < 0) ||
 	    read(dfd, buf, 4096) != 4096)
 		return 0;
 
-- 
2.32.0 (Apple Git-132)

[PATCH 05/14] mdadm/mdmon: fix coverity issue CHECKED_RETURN

[Xiao Ni]

It needs to check return values when functions have return value.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdmon.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index 5fdb5cdb5a49..49921076a37d 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -198,8 +198,12 @@ static void try_kill_monitor(pid_t pid, char *devname, int sock)
 	/* Wait for monitor to exit by reading from the socket, after
 	 * clearing the non-blocking flag */
 	fl = fcntl(sock, F_GETFL, 0);
+	if (fl < 0)
+		return;
+
 	fl &= ~O_NONBLOCK;
-	fcntl(sock, F_SETFL, fl);
+	if (fcntl(sock, F_SETFL, fl) < 0)
+		return;
 	n = read(sock, buf, 100);
 
 	/* If there is I/O going on it might took some time to get to
@@ -249,7 +253,10 @@ static int make_control_sock(char *devname)
 	listen(sfd, 10);
 	fl = fcntl(sfd, F_GETFL, 0);
 	fl |= O_NONBLOCK;
-	fcntl(sfd, F_SETFL, fl);
+	if (fcntl(sfd, F_SETFL, fl) < 0) {
+		close_fd(&sfd);
+		return -1;
+	}
 	return sfd;
 }
 
-- 
2.32.0 (Apple Git-132)

[PATCH 06/14] mdadm/mdmon: fix coverity issue RESOURCE_LEAK

[Xiao Ni]

Fix resource leak problem in mdmon.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdmon.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index 49921076a37d..f64940d576b4 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -458,22 +458,25 @@ static int mdmon(char *devnm, int must_fork, int takeover)
 	if (must_fork) {
 		if (pipe(pfd) != 0) {
 			pr_err("failed to create pipe\n");
+			close_fd(&mdfd);
 			return 1;
 		}
 		switch(fork()) {
 		case -1:
 			pr_err("failed to fork: %s\n", strerror(errno));
+			close_fd(&mdfd);
 			return 1;
 		case 0: /* child */
-			close(pfd[0]);
+			close_fd(&pfd[0]);
 			break;
 		default: /* parent */
-			close(pfd[1]);
+			close_fd(&pfd[1]);
 			if (read(pfd[0], &status, sizeof(status)) != sizeof(status)) {
 				wait(&status);
 				status = WEXITSTATUS(status);
 			}
-			close(pfd[0]);
+			close_fd(&pfd[0]);
+			close_fd(&mdfd);
 			return status;
 		}
 	} else
-- 
2.32.0 (Apple Git-132)

[PATCH 07/14] mdadm/mdopen: fix coverity issue CHECKED_RETURN

[Xiao Ni]

It needs to check return values when functions return value.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdopen.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mdopen.c b/mdopen.c
index eaa59b5925af..c9fda131558b 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -406,7 +406,11 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 				perror("chown");
 			if (chmod(devname, ci->mode))
 				perror("chmod");
-			stat(devname, &stb);
+			if (stat(devname, &stb) < 0) {
+				pr_err("failed to stat %s\n",
+						devname);
+				return -1;
+			}
 			add_dev(devname, &stb, 0, NULL);
 		}
 		if (use_mdp == 1)
-- 
2.32.0 (Apple Git-132)

[PATCH 08/14] mdadm/mdopen: fix coverity issue STRING_OVERFLOW

[Xiao Ni]

Fix string overflow problems in mdopen.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdopen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mdopen.c b/mdopen.c
index c9fda131558b..e49defb6744d 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -376,7 +376,7 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 
 	sprintf(devname, "/dev/%s", devnm);
 
-	if (dev && dev[0] == '/')
+	if (dev && dev[0] == '/' && strlen(dev) < 400)
 		strcpy(chosen, dev);
 	else if (cname[0] == 0)
 		strcpy(chosen, devname);
-- 
2.32.0 (Apple Git-132)

[PATCH 09/14] mdadm/mdstat: fix coverity issue CHECKED_RETURN

[Xiao Ni]

It needs to check return values when functions return value.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdstat.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/mdstat.c b/mdstat.c
index e233f094c480..930d59ee2325 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -146,8 +146,11 @@ struct mdstat_ent *mdstat_read(int hold, int start)
 		f = fopen("/proc/mdstat", "r");
 	if (f == NULL)
 		return NULL;
-	else
-		fcntl(fileno(f), F_SETFD, FD_CLOEXEC);
+
+	if (fcntl(fileno(f), F_SETFD, FD_CLOEXEC) < 0) {
+		fclose(f);
+		return NULL;
+	}
 
 	all = NULL;
 	end = &all;
@@ -281,7 +284,10 @@ struct mdstat_ent *mdstat_read(int hold, int start)
 	}
 	if (hold && mdstat_fd == -1) {
 		mdstat_fd = dup(fileno(f));
-		fcntl(mdstat_fd, F_SETFD, FD_CLOEXEC);
+		if (fcntl(mdstat_fd, F_SETFD, FD_CLOEXEC) < 0) {
+			fclose(f);
+			return NULL;
+		}
 	}
 	fclose(f);
 
-- 
2.32.0 (Apple Git-132)

[PATCH 10/14] mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER

[Xiao Ni]

Fix coverity problems in super0. It needs to check return value when
functions return value. And fix EVALUATION_ORDER problems in super0.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 super0.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/super0.c b/super0.c
index 9b8a1bd63bb7..6f43b1671d44 100644
--- a/super0.c
+++ b/super0.c
@@ -83,6 +83,9 @@ static void examine_super0(struct supertype *st, char *homehost)
 	int d;
 	int delta_extra = 0;
 	char *c;
+	unsigned long expected_csum = 0;
+
+	expected_csum = calc_sb0_csum(sb);
 
 	printf("          Magic : %08x\n", sb->md_magic);
 	printf("        Version : %d.%02d.%02d\n",
@@ -187,11 +190,11 @@ static void examine_super0(struct supertype *st, char *homehost)
 	printf("Working Devices : %d\n", sb->working_disks);
 	printf(" Failed Devices : %d\n", sb->failed_disks);
 	printf("  Spare Devices : %d\n", sb->spare_disks);
-	if (calc_sb0_csum(sb) == sb->sb_csum)
+	if (expected_csum == sb->sb_csum)
 		printf("       Checksum : %x - correct\n", sb->sb_csum);
 	else
 		printf("       Checksum : %x - expected %lx\n",
-		       sb->sb_csum, calc_sb0_csum(sb));
+		       sb->sb_csum, expected_csum);
 	printf("         Events : %llu\n",
 	       ((unsigned long long)sb->events_hi << 32) + sb->events_lo);
 	printf("\n");
@@ -1212,7 +1215,8 @@ static int locate_bitmap0(struct supertype *st, int fd, int node_num)
 
 	offset += MD_SB_BYTES;
 
-	lseek64(fd, offset, 0);
+	if (lseek64(fd, offset, 0) < 0)
+		return -1;
 	return 0;
 }
 
-- 
2.32.0 (Apple Git-132)

[PATCH 11/14] mdadm/super1: fix coverity issue CHECKED_RETURN

[Xiao Ni]

It needs to check return value when functions return value.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 super1.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/super1.c b/super1.c
index 81d29a652f36..4e4c7bfd15ae 100644
--- a/super1.c
+++ b/super1.c
@@ -260,7 +260,10 @@ static int aread(struct align_fd *afd, void *buf, int len)
 	n = read(afd->fd, b, iosize);
 	if (n <= 0)
 		return n;
-	lseek(afd->fd, len - n, 1);
+	if (lseek(afd->fd, len - n, 1) < 0) {
+		pr_err("lseek fails\n");
+		return -1;
+	}
 	if (n > len)
 		n = len;
 	memcpy(buf, b, n);
@@ -294,14 +297,20 @@ static int awrite(struct align_fd *afd, void *buf, int len)
 		n = read(afd->fd, b, iosize);
 		if (n <= 0)
 			return n;
-		lseek(afd->fd, -n, 1);
+		if (lseek(afd->fd, -n, 1) < 0) {
+			pr_err("lseek fails\n");
+			return -1;
+		}
 	}
 
 	memcpy(b, buf, len);
 	n = write(afd->fd, b, iosize);
 	if (n <= 0)
 		return n;
-	lseek(afd->fd, len - n, 1);
+	if (lseek(afd->fd, len - n, 1) < 0) {
+		pr_err("lseek fails\n");
+		return -1;
+	}
 	return len;
 }
 
@@ -2667,7 +2676,10 @@ static int locate_bitmap1(struct supertype *st, int fd, int node_num)
 	}
 	if (mustfree)
 		free(sb);
-	lseek64(fd, offset<<9, 0);
+	if (lseek64(fd, offset<<9, 0) < 0) {
+		pr_err("lseek fails\n");
+		ret = -1;
+	}
 	return ret;
 }
 
-- 
2.32.0 (Apple Git-132)

[PATCH 12/14] mdadm/super1: fix coverity issue DEADCODE

[Xiao Ni]

optimal_space is at most 2046. So space can't be larger than UINT16_MAX.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 super1.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/super1.c b/super1.c
index 4e4c7bfd15ae..24bc10269dbf 100644
--- a/super1.c
+++ b/super1.c
@@ -1466,8 +1466,6 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 						__le32_to_cpu(sb->chunksize));
 			if (space > optimal_space)
 				space = optimal_space;
-			if (space > UINT16_MAX)
-				space = UINT16_MAX;
 		}
 
 		sb->ppl.offset = __cpu_to_le16(offset);
-- 
2.32.0 (Apple Git-132)

[PATCH 13/14] mdadm/super1: fix coverity issue EVALUATION_ORDER

[Xiao Ni]

Fix evaluation order problems in super1.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 super1.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/super1.c b/super1.c
index 24bc10269dbf..243eeb1a0174 100644
--- a/super1.c
+++ b/super1.c
@@ -340,6 +340,9 @@ static void examine_super1(struct supertype *st, char *homehost)
 	unsigned long long sb_offset;
 	struct mdinfo info;
 	int inconsistent = 0;
+	unsigned int expected_csum = 0;
+
+	expected_csum = calc_sb_1_csum(sb);
 
 	printf("          Magic : %08x\n", __le32_to_cpu(sb->magic));
 	printf("        Version : 1");
@@ -507,13 +510,13 @@ static void examine_super1(struct supertype *st, char *homehost)
 		printf("\n");
 	}
 
-	if (calc_sb_1_csum(sb) == sb->sb_csum)
+	if (expected_csum == sb->sb_csum)
 		printf("       Checksum : %x - correct\n",
 		       __le32_to_cpu(sb->sb_csum));
 	else
 		printf("       Checksum : %x - expected %x\n",
 		       __le32_to_cpu(sb->sb_csum),
-		       __le32_to_cpu(calc_sb_1_csum(sb)));
+		       __le32_to_cpu(expected_csum));
 	printf("         Events : %llu\n",
 	       (unsigned long long)__le64_to_cpu(sb->events));
 	printf("\n");
-- 
2.32.0 (Apple Git-132)

[PATCH 14/14] mdadm/super1: fix coverity issue RESOURCE_LEAK

[Xiao Ni]

Fix resource leak problems in super1.c

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 super1.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/super1.c b/super1.c
index 243eeb1a0174..9c9c7dd14c15 100644
--- a/super1.c
+++ b/super1.c
@@ -923,10 +923,12 @@ static int examine_badblocks_super1(struct supertype *st, int fd, char *devname)
 	offset <<= 9;
 	if (lseek64(fd, offset, 0) < 0) {
 		pr_err("Cannot seek to bad-blocks list\n");
+		free(bbl);
 		return 1;
 	}
 	if (read(fd, bbl, size) != size) {
 		pr_err("Cannot read bad-blocks list\n");
+		free(bbl);
 		return 1;
 	}
 	/* 64bits per entry. 10 bits is block-count, 54 bits is block
@@ -947,6 +949,7 @@ static int examine_badblocks_super1(struct supertype *st, int fd, char *devname)
 
 		printf("%20llu for %d sectors\n", sector, count);
 	}
+	free(bbl);
 	return 0;
 }
 
-- 
2.32.0 (Apple Git-132)

Re: [PATCH V4 00/14] mdadm: fix coverity issues

[Paul Menzel]

Dear Xiao,


Thank you for taking care of these things. Some comments on minor things.


Am 26.07.24 um 09:14 schrieb Xiao Ni:

[…]

> Xiao Ni (14):
>    mdadm/Grow: fix coverity issue CHECKED_RETURN
>    mdadm/Grow: fix coverity issue RESOURCE_LEAK
>    mdadm/Grow: fix coverity issue STRING_OVERFLOW
>    mdadm/Incremental: fix coverity issues.

I’d remove the dot/period at the end

>    mdadm/mdmon: fix coverity issue CHECKED_RETURN
>    mdadm/mdmon: fix coverity issue RESOURCE_LEAK
>    mdadm/mdopen: fix coverity issue CHECKED_RETURN
>    mdadm/mdopen: fix coverity issue STRING_OVERFLOW
>    mdadm/mdstat: fix coverity issue CHECKED_RETURN
>    mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER
>    mdadm/super1: fix coverity issue CHECKED_RETURN
>    mdadm/super1: fix coverity issue DEADCODE
>    mdadm/super1: fix coverity issue EVALUATION_ORDER
>    mdadm/super1: fix coverity issue RESOURCE_LEAK

In my opinion, naming the tool reporting the issue in the commit message 
summary is not beneficial, and I’d prefer to have more detail on the 
change in there. The tool could be named/credited in the commit message 
body.


Kind regards,

Paul

Re: [PATCH V4 00/14] mdadm: fix coverity issues

[Mariusz Tkaczyk]

On Fri, 26 Jul 2024 10:01:29 +0200
Paul Menzel <pmenzel@molgen.mpg.de> wrote:

> In my opinion, naming the tool reporting the issue in the commit message 
> summary is not beneficial, and I’d prefer to have more detail on the 
> change in there. The tool could be named/credited in the commit message 
> body.

Hmm, I didn't put a huge patience into that, I accepted similar commits from
Nigel on GH, like this one:

https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=1b4b73fd535a6487075e98f620454ff2e13b5240

However, he used the exact problem description reported by the tool in commit
message. This is not exactly the same style.

I may looks like hypocrite, asking for changes from Xiao now.

For that reason I'm fine with current style but I doesn't mean that I disagree
with Paul. I agree with Paul.

Xiao, let me know if you would like to rework descriptions or you would
prefer me to pick it in this form.

Mariusz

Re: [PATCH V4 00/14] mdadm: fix coverity issues

[Mariusz Tkaczyk]

On Fri, 26 Jul 2024 15:14:02 +0800
Xiao Ni <xni@redhat.com> wrote:

> V2: replace close with close_fd and use is_fd_valid
> V3: Fix errors reported by checkpatch
> V4:
> don't need check fd is valid before close_fd
> replace strcat with snprintf
> replace dev_name with dev_path_name
> 
> Xiao Ni (14):
>   mdadm/Grow: fix coverity issue CHECKED_RETURN
>   mdadm/Grow: fix coverity issue RESOURCE_LEAK
>   mdadm/Grow: fix coverity issue STRING_OVERFLOW
>   mdadm/Incremental: fix coverity issues.
>   mdadm/mdmon: fix coverity issue CHECKED_RETURN
>   mdadm/mdmon: fix coverity issue RESOURCE_LEAK
>   mdadm/mdopen: fix coverity issue CHECKED_RETURN
>   mdadm/mdopen: fix coverity issue STRING_OVERFLOW
>   mdadm/mdstat: fix coverity issue CHECKED_RETURN
>   mdadm/super0: fix coverity issue CHECKED_RETURN and EVALUATION_ORDER
>   mdadm/super1: fix coverity issue CHECKED_RETURN
>   mdadm/super1: fix coverity issue DEADCODE
>   mdadm/super1: fix coverity issue EVALUATION_ORDER
>   mdadm/super1: fix coverity issue RESOURCE_LEAK
> 
>  Grow.c        | 87 ++++++++++++++++++++++++++++++++++++++++-----------
>  Incremental.c | 20 ++++++------
>  mdmon.c       | 20 +++++++++---
>  mdopen.c      |  8 +++--
>  mdstat.c      | 12 +++++--
>  super0.c      | 10 ++++--
>  super1.c      | 32 ++++++++++++++-----
>  7 files changed, 139 insertions(+), 50 deletions(-)
> 

Applied!
Sorry for the delay. I was few days out of office!

Mariusz