[PATCH 0/6] SAST fixes

[PATCH 0/6] SAST fixes

[Mateusz Kusiak]

This patchset contains minor fixes for issues reported by SAST tool.

Mateusz Kusiak (6):
  Create: add_disk_to_super() fix resource leak
  mdadm: signal_s() init variables
  Monitor: open file before check in check_one_sharer()
  Grow: remove dead condition in Grow_reshape()
  super1: check fd before passing to get_dev_size() in add_to_super1()
  mdmon: refactor md device name check in main()

 Create.c  |  6 +++++-
 Grow.c    |  6 +-----
 Monitor.c | 13 +++++--------
 mdadm.h   |  5 ++---
 mdmon.c   | 21 +++++++++++----------
 super1.c  |  5 ++++-
 6 files changed, 28 insertions(+), 28 deletions(-)

-- 
2.35.3

[PATCH 1/6] Create: add_disk_to_super() fix resource leak

[Mateusz Kusiak]

Fixes resource leak in add_disk_to_super().

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 Create.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Create.c b/Create.c
index 8082f54a8fdc..7e9170b6a1ac 100644
--- a/Create.c
+++ b/Create.c
@@ -279,8 +279,10 @@ static int add_disk_to_super(int mdfd, struct shape *s, struct context *c,
 			       dv->devname);
 			return 1;
 		}
-		if (!fstat_is_blkdev(fd, dv->devname, &rdev))
+		if (!fstat_is_blkdev(fd, dv->devname, &rdev)) {
+			close(fd);
 			return 1;
+		}
 		info->disk.major = major(rdev);
 		info->disk.minor = minor(rdev);
 	}
@@ -289,6 +291,7 @@ static int add_disk_to_super(int mdfd, struct shape *s, struct context *c,
 	if (st->ss->add_to_super(st, &info->disk, fd, dv->devname,
 				 dv->data_offset)) {
 		ioctl(mdfd, STOP_ARRAY, NULL);
+		close(fd);
 		return 1;
 	}
 	st->ss->getinfo_super(st, info, NULL);
@@ -297,6 +300,7 @@ static int add_disk_to_super(int mdfd, struct shape *s, struct context *c,
 		*zero_pid = write_zeroes_fork(fd, s, st, dv);
 		if (*zero_pid <= 0) {
 			ioctl(mdfd, STOP_ARRAY, NULL);
+			close(fd);
 			return 1;
 		}
 	}
-- 
2.35.3

[PATCH 2/6] mdadm: signal_s() init variables

[Mateusz Kusiak]

Init sigaction structs in signal_s().
This approach might throw warnings for GCC 4.x and lower.

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 mdadm.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mdadm.h b/mdadm.h
index 1f28b3e754be..75c887e4c64c 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1856,11 +1856,10 @@ static inline char *to_subarray(struct mdstat_ent *ent, char *container)
  */
 static inline sighandler_t signal_s(int sig, sighandler_t handler)
 {
-	struct sigaction new_act;
-	struct sigaction old_act;
+	struct sigaction new_act = {0};
+	struct sigaction old_act = {0};
 
 	new_act.sa_handler = handler;
-	new_act.sa_flags = 0;
 
 	if (sigaction(sig, &new_act, &old_act) == 0)
 		return old_act.sa_handler;
-- 
2.35.3

[PATCH 3/6] Monitor: open file before check in check_one_sharer()

[Mateusz Kusiak]

Open file before performing checks in check_one_sharer() to avoid
file tampering.
Remove redundant access check.

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 Monitor.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 824a69fc6b79..7cee95d4487a 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -451,20 +451,17 @@ static int check_one_sharer(int scan)
 		return 2;
 	}
 
-	if (access(AUTOREBUILD_PID_PATH, F_OK) != 0)
-		return 0;
-
-	if (!is_file(AUTOREBUILD_PID_PATH)) {
-		pr_err("%s is not a regular file.\n", AUTOREBUILD_PID_PATH);
-		return 2;
-	}
-
 	fp = fopen(AUTOREBUILD_PID_PATH, "r");
 	if (!fp) {
 		pr_err("Cannot open %s file.\n", AUTOREBUILD_PID_PATH);
 		return 2;
 	}
 
+	if (!is_file(AUTOREBUILD_PID_PATH)) {
+		pr_err("%s is not a regular file.\n", AUTOREBUILD_PID_PATH);
+		return 2;
+	}
+
 	if (fscanf(fp, "%d", &pid) != 1) {
 		pr_err("Cannot read pid from %s file.\n", AUTOREBUILD_PID_PATH);
 		fclose(fp);
-- 
2.35.3

[PATCH 4/6] Grow: remove dead condition in Grow_reshape()

[Mateusz Kusiak]

Remove dead "if" condition from Grow_reshape(). Sysfs read check is
performed earlier in the code.

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 Grow.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Grow.c b/Grow.c
index f95dae82ef0d..dad7294dac32 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2097,11 +2097,7 @@ int Grow_reshape(char *devname, int fd,
 			/* got truncated to 32bit, write to
 			 * component_size instead
 			 */
-			if (sra)
-				rv = sysfs_set_num(sra, NULL,
-						   "component_size", s->size);
-			else
-				rv = -1;
+			rv = sysfs_set_num(sra, NULL, "component_size", s->size);
 		} else {
 			rv = md_set_array_info(fd, &array);
 
-- 
2.35.3

[PATCH 5/6] super1: check fd before passing to get_dev_size() in add_to_super1()

[Mateusz Kusiak]

Check if file descriptor is valid before passing it to get_dev_size() in
add_to_super().

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 super1.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/super1.c b/super1.c
index 871d19f0398c..5439b7bb1240 100644
--- a/super1.c
+++ b/super1.c
@@ -1752,7 +1752,10 @@ static int add_to_super1(struct supertype *st, mdu_disk_info_t *dk,
 	di->devname = devname;
 	di->disk = *dk;
 	di->data_offset = data_offset;
-	get_dev_size(fd, NULL, &di->dev_size);
+
+	if (is_fd_valid(fd))
+		get_dev_size(fd, NULL, &di->dev_size);
+
 	di->next = NULL;
 	*dip = di;
 
-- 
2.35.3

[PATCH 6/6] mdmon: refactor md device name check in main()

[Mateusz Kusiak]

Refactor mdmon main function to verify if fd is valid prior to checking
device name. This is due to static code analysis complaining after
change b938519e7719 ("util: remove obsolete code from get_md_name").

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 mdmon.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/mdmon.c b/mdmon.c
index a2038fe6c35f..5fdb5cdb5a49 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -302,12 +302,12 @@ static int mdmon(char *devnm, int must_fork, int takeover);
 int main(int argc, char *argv[])
 {
 	char *container_name = NULL;
-	char *devnm = NULL;
 	int status = 0;
 	int opt;
 	int all = 0;
 	int takeover = 0;
 	int dofork = 1;
+	int mdfd = -1;
 	bool help = false;
 	static struct option options[] = {
 		{"all", 0, NULL, 'a'},
@@ -410,19 +410,20 @@ int main(int argc, char *argv[])
 		free_mdstat(mdstat);
 
 		return status;
-	} else {
-		int mdfd = open_mddev(container_name, 0);
-		devnm = fd2devnm(mdfd);
+	}
+
+	mdfd = open_mddev(container_name, 0);
+	if (is_fd_valid(mdfd)) {
+		char *devnm = fd2devnm(mdfd);
 
 		close(mdfd);
-	}
 
-	if (!devnm) {
-		pr_err("%s is not a valid md device name\n",
-			container_name);
-		return 1;
+		if (devnm)
+			return mdmon(devnm, dofork && do_fork(), takeover);
 	}
-	return mdmon(devnm, dofork && do_fork(), takeover);
+
+	pr_err("%s is not a valid md device name\n", container_name);
+	return 1;
 }
 
 static int mdmon(char *devnm, int must_fork, int takeover)
-- 
2.35.3

Re: [PATCH 0/6] SAST fixes

[Mariusz Tkaczyk]

On Tue, 20 Feb 2024 11:56:06 +0100
Mateusz Kusiak <mateusz.kusiak@intel.com> wrote:

> This patchset contains minor fixes for issues reported by SAST tool.
> 
> Mateusz Kusiak (6):
>   Create: add_disk_to_super() fix resource leak
>   mdadm: signal_s() init variables
>   Monitor: open file before check in check_one_sharer()
>   Grow: remove dead condition in Grow_reshape()
>   super1: check fd before passing to get_dev_size() in add_to_super1()
>   mdmon: refactor md device name check in main()
> 
>  Create.c  |  6 +++++-
>  Grow.c    |  6 +-----
>  Monitor.c | 13 +++++--------
>  mdadm.h   |  5 ++---
>  mdmon.c   | 21 +++++++++++----------
>  super1.c  |  5 ++++-
>  6 files changed, 28 insertions(+), 28 deletions(-)
> 

All applied! 

Thanks,
Mariusz