To meet requirements of Common Criteria certification vulnerablility
assessment. Static code analysis has been run and found the following
errors:
check_return: Calling "fstat(fd, &dstb)" without checking return value.
This library function may fail and return an error code.
Changes are to add a test to the return value from fstat calls.
V2 - Assemble.c,Dump.c,Grow.c combined error paths into one path.
Signed-off-by: Nigel Croxon <ncroxon@xxxxxxxxxx>
---
Assemble.c | 49 ++++++++++++++++++++++---------------------------
Dump.c | 30 +++++++++++++++++-------------
Grow.c | 16 ++++++++++++----
config.c | 5 ++++-
managemon.c | 9 ++++++---
mdadm.h | 2 +-
mdstat.c | 10 ++++++++--
super-ddf.c | 11 +++++++++--
super-intel.c | 11 +++++++++--
9 files changed, 88 insertions(+), 55 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 0df4624..1bd16f0 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -649,7 +649,11 @@ static int load_devices(struct devs *devices, char *devmap,
/* prepare useful information in info structures */
struct stat stb2;
int err;
- fstat(mdfd, &stb2);
+
+ if (fstat(mdfd, &stb2) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ goto out;
+ }
if (strcmp(c->update, "uuid") == 0 && !ident->uuid_set)
random_uuid((__u8 *)ident->uuid);
@@ -657,10 +661,7 @@ static int load_devices(struct devs *devices, char *devmap,
if (strcmp(c->update, "ppl") == 0 &&
ident->bitmap_fd >= 0) {
pr_err("PPL is not compatible with bitmap\n");
- close(mdfd);
- free(devices);
- free(devmap);
- return -1;
+ goto out;
}
dfd = dev_open(devname,
@@ -673,13 +674,9 @@ static int load_devices(struct devs *devices, char *devmap,
devname);
if (dfd >= 0)
close(dfd);
- close(mdfd);
- free(devices);
- free(devmap);
tst->ss->free_super(tst);
free(tst);
- *stp = st;
- return -1;
+ goto next;
}
tst->ss->getinfo_super(tst, content, devmap + devcnt * content->array.raid_disks);
@@ -712,13 +709,8 @@ static int load_devices(struct devs *devices, char *devmap,
pr_err("--update=%s not understood for %s metadata\n",
c->update, tst->ss->name);
tst->ss->free_super(tst);
- free(tst);
- close(mdfd);
close(dfd);
- free(devices);
- free(devmap);
- *stp = st;
- return -1;
+ goto next;
}
if (strcmp(c->update, "uuid")==0 &&
!ident->uuid_set) {
@@ -749,18 +741,17 @@ static int load_devices(struct devs *devices, char *devmap,
devname);
if (dfd >= 0)
close(dfd);
- close(mdfd);
- free(devices);
- free(devmap);
tst->ss->free_super(tst);
free(tst);
- *stp = st;
- return -1;
+ goto next;
}
tst->ss->getinfo_super(tst, content, devmap + devcnt * content->array.raid_disks);
}
- fstat(dfd, &stb);
+ if (fstat(dfd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s - aborting\n",devname, strerror(errno));
+ goto next;
+ }
close(dfd);
if (c->verbose > 0)
@@ -840,11 +831,7 @@ static int load_devices(struct devs *devices, char *devmap,
inargv ? "the list" :
"the\n DEVICE list in mdadm.conf"
);
- close(mdfd);
- free(devices);
- free(devmap);
- *stp = st;
- return -1;
+ goto next;
}
if (best[i] == -1 || (devices[best[i]].i.events
< devices[devcnt].i.events))
@@ -860,6 +847,14 @@ static int load_devices(struct devs *devices, char *devmap,
*bestp = best;
*stp = st;
return devcnt;
+
+next:
+ *stp = st;
+out:
+ close(mdfd);
+ free(devices);
+ free(devmap);
+ return -1;
}
static int force_array(struct mdinfo *content,
diff --git a/Dump.c b/Dump.c
index 736bcb6..6881dc1 100644
--- a/Dump.c
+++ b/Dump.c
@@ -95,26 +95,21 @@ int Dump_metadata(char *dev, char *dir, struct context *c,
if (ftruncate(fl, size) < 0) {
pr_err("failed to set size of dump file: %s\n",
strerror(errno));
- close(fd);
- close(fl);
- free(fname);
- return 1;
+ goto out;
}
if (st->ss->copy_metadata(st, fd, fl) != 0) {
pr_err("Failed to copy metadata from %s to %s\n",
dev, fname);
- close(fd);
- close(fl);
unlink(fname);
- free(fname);
- return 1;
+ goto out;
}
if (c->verbose >= 0)
printf("%s saved as %s.\n", dev, fname);
- fstat(fd, &dstb);
- close(fd);
- close(fl);
+ if (fstat(fd, &dstb) != 0) {
+ pr_err("fstat failed from %s: %s\n",fname, strerror(errno));
+ goto out;
+ }
if ((dstb.st_mode & S_IFMT) != S_IFBLK) {
/* Not a block device, so cannot create links */
free(fname);
@@ -153,6 +148,12 @@ int Dump_metadata(char *dev, char *dir, struct context *c,
closedir(dirp);
free(fname);
return 0;
+
+out:
+ close(fd);
+ close(fl);
+ free(fname);
+ return 1;
}
int Restore_metadata(char *dev, char *dir, struct context *c,
@@ -200,8 +201,11 @@ int Restore_metadata(char *dev, char *dir, struct context *c,
char *chosen = NULL;
unsigned int chosen_inode = 0;
- fstat(fd, &dstb);
-
+ if (fstat(fd, &dstb) != 0) {
+ pr_err("fstat failed for %s: %s\n",dev, strerror(errno));
+ close(fd);
+ return 1;
+ }
while (d && (de = readdir(d)) != NULL) {
if (de->d_name[0] == '.')
continue;
diff --git a/Grow.c b/Grow.c
index 7506ab4..1f3b0c1 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1163,13 +1163,18 @@ int reshape_open_backup_file(char *backup_file,
* way this will not notice, but it is better than
* nothing.
*/
- fstat(*fdlist, &stb);
+ if (fstat(*fdlist, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ goto out;
+ }
dev = stb.st_dev;
- fstat(fd, &stb);
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ goto out;
+ }
if (stb.st_rdev == dev) {
pr_err("backup file must NOT be on the array being reshaped.\n");
- close(*fdlist);
- return 0;
+ goto out;
}
memset(buf, 0, 512);
@@ -1195,6 +1200,9 @@ int reshape_open_backup_file(char *backup_file,
}
return 1;
+out:
+ close(*fdlist);
+ return 0;
}
unsigned long compute_backup_blocks(int nchunk, int ochunk,
diff --git a/config.c b/config.c
index 9c72545..ad0e02f 100644
--- a/config.c
+++ b/config.c
@@ -803,7 +803,10 @@ void conf_file_or_dir(FILE *f)
struct dirent *dp;
struct fname *list = NULL;
- fstat(fileno(f), &st);
+ if (fstat(fileno(f), &st) != 0) {
+ pr_err("fstat failed: %s\n", strerror(errno));
+ return;
+ }
if (S_ISREG(st.st_mode))
conf_file(f);
else if (!S_ISDIR(st.st_mode))
diff --git a/managemon.c b/managemon.c
index 200cf83..d943f2c 100644
--- a/managemon.c
+++ b/managemon.c
@@ -894,6 +894,7 @@ void do_manager(struct supertype *container)
{
struct mdstat_ent *mdstat;
sigset_t set;
+ int rtn;
sigprocmask(SIG_UNBLOCK, NULL, &set);
sigdelset(&set, SIGUSR1);
@@ -926,9 +927,11 @@ void do_manager(struct supertype *container)
if (sigterm)
wakeup_monitor();
- if (update_queue == NULL)
- mdstat_wait_fd(container->sock, &set);
- else
+ if (update_queue == NULL) {
+ rtn = mdstat_wait_fd(container->sock, &set);
+ if (rtn)
+ exit(0);
+ } else
/* If an update is happening, just wait for signal */
pselect(0, NULL, NULL, NULL, NULL, &set);
} while(1);
diff --git a/mdadm.h b/mdadm.h
index 8f8841d..1273029 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -637,7 +637,7 @@ extern struct mdstat_ent *mdstat_read(int hold, int start);
extern void mdstat_close(void);
extern void free_mdstat(struct mdstat_ent *ms);
extern int mdstat_wait(int seconds);
-extern void mdstat_wait_fd(int fd, const sigset_t *sigmask);
+extern int mdstat_wait_fd(int fd, const sigset_t *sigmask);
extern int mddev_busy(char *devnm);
extern struct mdstat_ent *mdstat_by_component(char *name);
extern struct mdstat_ent *mdstat_by_subdev(char *subdev, char *container);
diff --git a/mdstat.c b/mdstat.c
index 2fd792c..8664c0e 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -336,7 +336,7 @@ int mdstat_wait(int seconds)
return select(maxfd + 1, NULL, NULL, &fds, &tm);
}
-void mdstat_wait_fd(int fd, const sigset_t *sigmask)
+int mdstat_wait_fd(int fd, const sigset_t *sigmask)
{
fd_set fds, rfds;
int maxfd = 0;
@@ -348,7 +348,11 @@ void mdstat_wait_fd(int fd, const sigset_t *sigmask)
if (fd >= 0) {
struct stat stb;
- fstat(fd, &stb);
+
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed: %s\n", strerror(errno));
+ return 1;
+ }
if ((stb.st_mode & S_IFMT) == S_IFREG)
/* Must be a /proc or /sys fd, so expect
* POLLPRI
@@ -367,6 +371,8 @@ void mdstat_wait_fd(int fd, const sigset_t *sigmask)
pselect(maxfd + 1, &rfds, NULL, &fds,
NULL, sigmask);
+
+ return 0;
}
int mddev_busy(char *devnm)
diff --git a/super-ddf.c b/super-ddf.c
index dc8e512..3c9eadf 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1060,7 +1060,11 @@ static int load_ddf_local(int fd, struct ddf_super *super,
0);
dl->devname = devname ? xstrdup(devname) : NULL;
- fstat(fd, &stb);
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ free(dl);
+ return 1;
+ }
dl->major = major(stb.st_rdev);
dl->minor = minor(stb.st_rdev);
dl->next = super->dlist;
@@ -2854,7 +2858,10 @@ static int add_to_super_ddf(struct supertype *st,
/* This is device numbered dk->number. We need to create
* a phys_disk entry and a more detailed disk_data entry.
*/
- fstat(fd, &stb);
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ return 1;
+ }
n = find_unused_pde(ddf);
if (n == DDF_NOTFOUND) {
pr_err("No free slot in array, cannot add disk\n");
diff --git a/super-intel.c b/super-intel.c
index 83ddc00..1836be7 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4164,7 +4164,11 @@ load_imsm_disk(int fd, struct intel_super *super, char *devname, int keep_fd)
dl = xcalloc(1, sizeof(*dl));
- fstat(fd, &stb);
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ free(dl);
+ return 2;
+ }
dl->major = major(stb.st_rdev);
dl->minor = minor(stb.st_rdev);
dl->next = super->disks;
@@ -5910,7 +5914,10 @@ static int add_to_super_imsm(struct supertype *st, mdu_disk_info_t *dk,
if (super->current_vol >= 0)
return add_to_super_imsm_volume(st, dk, fd, devname);
- fstat(fd, &stb);
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n",devname, strerror(errno));
+ return 1;
+ }
dd = xcalloc(sizeof(*dd), 1);
dd->major = major(stb.st_rdev);
dd->minor = minor(stb.st_rdev);
--
2.29.2
