You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
474 lines
13 KiB
474 lines
13 KiB
From 96b8035a09b6449ea99f2eb91f9ba4f6912e5bd6 Mon Sep 17 00:00:00 2001
|
|
From: Nigel Croxon <ncroxon@redhat.com>
|
|
Date: Tue, 2 Jul 2024 10:11:26 -0400
|
|
Subject: [PATCH 116/157] mdadm: super-ddf.c fix coverity issues
|
|
|
|
Fixing the following coding errors the coverity tools found:
|
|
|
|
* Calling "lseek64" without checking return value. This library function may
|
|
fail and return an error code.
|
|
* Overrunning array "anchor->pad2" of 3 bytes by passing it to a function
|
|
which accesses it at byte offset 398 using argument "399UL".
|
|
* Event leaked_storage: Variable "sra" going out of scope leaks the storage
|
|
it points to.
|
|
* Event leaked_storage: Variable "super" going out of scope leaks the storage
|
|
it points to.
|
|
* Event leaked_handle: Handle variable "dfd" going out of scope leaks the
|
|
handle.
|
|
* Event leaked_storage: Variable "dl1" going out of scope leaks the storage
|
|
it points to
|
|
* Event leaked_handle: Handle variable "cfd" going out of scope leaks the
|
|
handle.
|
|
* Variable "avail" going out of scope leaks the storage it points to.
|
|
* Passing unterminated string "super->anchor.revision" to "fprintf", which
|
|
expects a null-terminated string.
|
|
* You might overrun the 32-character fixed-size string "st->container_devnm"
|
|
by copying the return value of "fd2devnm" without checking the length.
|
|
* Event fixed_size_dest: You might overrun the 33-character fixed-size string
|
|
"dev->name" by copying "(*d).devname" without checking the length.
|
|
* Event uninit_use_in_call: Using uninitialized value "info.array.raid_disks"
|
|
when calling "getinfo_super_ddf"
|
|
|
|
V2: clean up validate_geometry_ddf() routine with Mariusz Tkaczyk recommendations.
|
|
V3: clean up spaces with Blazej Kucman recommendations.
|
|
V4: clean up recommended by Mariusz Tkaczyk.
|
|
V5: clean up recommended by Mariusz Tkaczyk.
|
|
|
|
Signed-off-by: Nigel Croxon <ncroxon@redhat.com>
|
|
---
|
|
super-ddf.c | 172 +++++++++++++++++++++++++++++++++++-----------------
|
|
1 file changed, 115 insertions(+), 57 deletions(-)
|
|
|
|
diff --git a/super-ddf.c b/super-ddf.c
|
|
index 311001c1..d870102d 100644
|
|
--- a/super-ddf.c
|
|
+++ b/super-ddf.c
|
|
@@ -809,7 +809,7 @@ static int load_ddf_header(int fd, unsigned long long lba,
|
|
if (lba >= size-1)
|
|
return 0;
|
|
|
|
- if (lseek64(fd, lba<<9, 0) < 0)
|
|
+ if (lseek64(fd, lba << 9, 0) == -1L)
|
|
return 0;
|
|
|
|
if (read(fd, hdr, 512) != 512)
|
|
@@ -828,8 +828,7 @@ static int load_ddf_header(int fd, unsigned long long lba,
|
|
!be64_eq(anchor->primary_lba, hdr->primary_lba) ||
|
|
!be64_eq(anchor->secondary_lba, hdr->secondary_lba) ||
|
|
hdr->type != type ||
|
|
- memcmp(anchor->pad2, hdr->pad2, 512 -
|
|
- offsetof(struct ddf_header, pad2)) != 0) {
|
|
+ memcmp(anchor->pad2, hdr->pad2, sizeof(anchor->pad2)) != 0) {
|
|
pr_err("header mismatch\n");
|
|
return 0;
|
|
}
|
|
@@ -863,7 +862,7 @@ static void *load_section(int fd, struct ddf_super *super, void *buf,
|
|
else
|
|
offset += be64_to_cpu(super->active->secondary_lba);
|
|
|
|
- if ((unsigned long long)lseek64(fd, offset<<9, 0) != (offset<<9)) {
|
|
+ if ((unsigned long long)lseek64(fd, offset << 9, 0) != (offset << 9)) {
|
|
if (dofree)
|
|
free(buf);
|
|
return NULL;
|
|
@@ -882,7 +881,7 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
|
|
|
|
get_dev_size(fd, NULL, &dsize);
|
|
|
|
- if (lseek64(fd, dsize-512, 0) < 0) {
|
|
+ if (lseek64(fd, dsize - 512, 0) == -1L) {
|
|
if (devname)
|
|
pr_err("Cannot seek to anchor block on %s: %s\n",
|
|
devname, strerror(errno));
|
|
@@ -909,8 +908,7 @@ static int load_ddf_headers(int fd, struct ddf_super *super, char *devname)
|
|
if (memcmp(super->anchor.revision, DDF_REVISION_0, 8) != 0 &&
|
|
memcmp(super->anchor.revision, DDF_REVISION_2, 8) != 0) {
|
|
if (devname)
|
|
- pr_err("can only support super revision %.8s and earlier, not %.8s on %s\n",
|
|
- DDF_REVISION_2, super->anchor.revision,devname);
|
|
+ pr_err("The DDF revision on %s\n is not supported", devname);
|
|
return 2;
|
|
}
|
|
super->active = NULL;
|
|
@@ -1610,6 +1608,7 @@ static unsigned int get_vd_num_of_subarray(struct supertype *st)
|
|
return DDF_NOTFOUND;
|
|
}
|
|
|
|
+ sysfs_free(sra);
|
|
return vcnum;
|
|
}
|
|
|
|
@@ -1617,11 +1616,11 @@ static void brief_examine_super_ddf(struct supertype *st, int verbose)
|
|
{
|
|
/* We just write a generic DDF ARRAY entry
|
|
*/
|
|
- struct mdinfo info;
|
|
+ struct mdinfo info = {0};
|
|
char nbuf[64];
|
|
+
|
|
getinfo_super_ddf(st, &info, NULL);
|
|
fname_from_uuid(&info, nbuf);
|
|
-
|
|
printf("ARRAY metadata=ddf UUID=%s\n", nbuf + 5);
|
|
}
|
|
|
|
@@ -1631,9 +1630,10 @@ static void brief_examine_subarrays_ddf(struct supertype *st, int verbose)
|
|
* by uuid and member by unit number and uuid.
|
|
*/
|
|
struct ddf_super *ddf = st->sb;
|
|
- struct mdinfo info;
|
|
+ struct mdinfo info = {0};
|
|
unsigned int i;
|
|
char nbuf[64];
|
|
+
|
|
getinfo_super_ddf(st, &info, NULL);
|
|
fname_from_uuid(&info, nbuf);
|
|
|
|
@@ -1658,8 +1658,9 @@ static void brief_examine_subarrays_ddf(struct supertype *st, int verbose)
|
|
|
|
static void export_examine_super_ddf(struct supertype *st)
|
|
{
|
|
- struct mdinfo info;
|
|
+ struct mdinfo info = {0};
|
|
char nbuf[64];
|
|
+
|
|
getinfo_super_ddf(st, &info, NULL);
|
|
fname_from_uuid(&info, nbuf);
|
|
printf("MD_METADATA=ddf\n");
|
|
@@ -1692,10 +1693,12 @@ static int copy_metadata_ddf(struct supertype *st, int from, int to)
|
|
if (!get_dev_size(from, NULL, &dsize))
|
|
goto err;
|
|
|
|
- if (lseek64(from, dsize-512, 0) < 0)
|
|
+ if (lseek64(from, dsize - 512, 0) == -1L)
|
|
goto err;
|
|
+
|
|
if (read(from, buf, 512) != 512)
|
|
goto err;
|
|
+
|
|
ddf = buf;
|
|
if (!be32_eq(ddf->magic, DDF_HEADER_MAGIC) ||
|
|
!be32_eq(calc_crc(ddf, 512), ddf->crc) ||
|
|
@@ -1711,9 +1714,9 @@ static int copy_metadata_ddf(struct supertype *st, int from, int to)
|
|
|
|
bytes = dsize - offset;
|
|
|
|
- if (lseek64(from, offset, 0) < 0 ||
|
|
- lseek64(to, offset, 0) < 0)
|
|
+ if (lseek64(from, offset, 0) == -1L || lseek64(to, offset, 0) == -1L)
|
|
goto err;
|
|
+
|
|
while (written < bytes) {
|
|
int n = bytes - written;
|
|
if (n > 4096)
|
|
@@ -1795,6 +1798,7 @@ static void brief_detail_super_ddf(struct supertype *st, char *subarray)
|
|
char nbuf[64];
|
|
struct ddf_super *ddf = st->sb;
|
|
unsigned int vcnum = get_vd_num_of_subarray(st);
|
|
+
|
|
if (vcnum == DDF_CONTAINER)
|
|
uuid_from_super_ddf(st, info.uuid);
|
|
else if (vcnum == DDF_NOTFOUND)
|
|
@@ -2971,7 +2975,9 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type)
|
|
header->openflag = 1;
|
|
header->crc = calc_crc(header, 512);
|
|
|
|
- lseek64(fd, sector<<9, 0);
|
|
+ if (lseek64(fd, sector << 9, 0) == -1L)
|
|
+ goto out;
|
|
+
|
|
if (write(fd, header, 512) < 0)
|
|
goto out;
|
|
|
|
@@ -2982,6 +2988,7 @@ static int __write_ddf_structure(struct dl *d, struct ddf_super *ddf, __u8 type)
|
|
ddf->phys->crc = calc_crc(ddf->phys, ddf->pdsize);
|
|
if (write(fd, ddf->phys, ddf->pdsize) < 0)
|
|
goto out;
|
|
+
|
|
ddf->virt->crc = calc_crc(ddf->virt, ddf->vdsize);
|
|
if (write(fd, ddf->virt, ddf->vdsize) < 0)
|
|
goto out;
|
|
@@ -3035,7 +3042,9 @@ out:
|
|
header->openflag = 0;
|
|
header->crc = calc_crc(header, 512);
|
|
|
|
- lseek64(fd, sector<<9, 0);
|
|
+ if (lseek64(fd, sector << 9, 0) == -1L)
|
|
+ return 0;
|
|
+
|
|
if (write(fd, header, 512) < 0)
|
|
ret = 0;
|
|
|
|
@@ -3088,7 +3097,9 @@ static int _write_super_to_disk(struct ddf_super *ddf, struct dl *d)
|
|
if (!__write_ddf_structure(d, ddf, DDF_HEADER_SECONDARY))
|
|
return 0;
|
|
|
|
- lseek64(fd, (size-1)*512, SEEK_SET);
|
|
+ if (lseek64(fd, (size - 1) * 512, SEEK_SET) == -1L)
|
|
+ return 0;
|
|
+
|
|
if (write(fd, &ddf->anchor, 512) < 0)
|
|
return 0;
|
|
|
|
@@ -3299,9 +3310,10 @@ static int validate_geometry_ddf(struct supertype *st,
|
|
char *dev, unsigned long long *freesize,
|
|
int consistency_policy, int verbose)
|
|
{
|
|
- int fd;
|
|
- struct mdinfo *sra;
|
|
+ struct mdinfo *sra = NULL;
|
|
+ int ret = 1;
|
|
int cfd;
|
|
+ int fd;
|
|
|
|
/* ddf potentially supports lots of things, but it depends on
|
|
* what devices are offered (and maybe kernel version?)
|
|
@@ -3369,7 +3381,7 @@ static int validate_geometry_ddf(struct supertype *st,
|
|
* Later we should check for a BVD and make an SVD.
|
|
*/
|
|
fd = open(dev, O_RDONLY|O_EXCL, 0);
|
|
- if (fd >= 0) {
|
|
+ if (is_fd_valid(fd)) {
|
|
close(fd);
|
|
/* Just a bare device, no good to us */
|
|
if (verbose)
|
|
@@ -3377,44 +3389,58 @@ static int validate_geometry_ddf(struct supertype *st,
|
|
dev);
|
|
return 0;
|
|
}
|
|
+
|
|
if (errno != EBUSY || (fd = open(dev, O_RDONLY, 0)) < 0) {
|
|
if (verbose)
|
|
pr_err("ddf: Cannot open %s: %s\n",
|
|
dev, strerror(errno));
|
|
return 0;
|
|
}
|
|
+
|
|
/* Well, it is in use by someone, maybe a 'ddf' container. */
|
|
cfd = open_container(fd);
|
|
- if (cfd < 0) {
|
|
- close(fd);
|
|
+ close(fd);
|
|
+
|
|
+ if (!is_fd_valid(cfd)) {
|
|
if (verbose)
|
|
- pr_err("ddf: Cannot use %s: %s\n",
|
|
- dev, strerror(EBUSY));
|
|
+ pr_err("ddf: Cannot use %s\n", dev);
|
|
return 0;
|
|
}
|
|
+
|
|
sra = sysfs_read(cfd, NULL, GET_VERSION);
|
|
- close(fd);
|
|
- if (sra && sra->array.major_version == -1 &&
|
|
- strcmp(sra->text_version, "ddf") == 0) {
|
|
+ if (!sra) {
|
|
+ pr_err("Cannot read sysfs for /dev/%s\n", fd2kname(cfd));
|
|
+ goto error;
|
|
+ }
|
|
+
|
|
+ if (sra->array.major_version == -1 && strcmp(sra->text_version, "ddf") == 0) {
|
|
/* This is a member of a ddf container. Load the container
|
|
* and try to create a bvd
|
|
*/
|
|
- struct ddf_super *ddf;
|
|
+ struct ddf_super *ddf = NULL;
|
|
+
|
|
if (load_super_ddf_all(st, cfd, (void **)&ddf, NULL) == 0) {
|
|
st->sb = ddf;
|
|
- strcpy(st->container_devnm, fd2devnm(cfd));
|
|
+ snprintf(st->container_devnm, sizeof(st->container_devnm),
|
|
+ "%s", fd2kname(cfd));
|
|
close(cfd);
|
|
- return validate_geometry_ddf_bvd(st, level, layout,
|
|
- raiddisks, chunk, size,
|
|
- data_offset,
|
|
- dev, freesize,
|
|
- verbose);
|
|
+ free(sra);
|
|
+
|
|
+ return validate_geometry_ddf_bvd(st, level, layout, raiddisks,
|
|
+ chunk, size, data_offset, dev,
|
|
+ freesize, verbose);
|
|
}
|
|
- close(cfd);
|
|
- } else /* device may belong to a different container */
|
|
- return 0;
|
|
+ free(ddf);
|
|
+ }
|
|
|
|
- return 1;
|
|
+ /* device may belong to a different container */
|
|
+ ret = 0;
|
|
+
|
|
+error:
|
|
+ free(sra);
|
|
+ close(cfd);
|
|
+
|
|
+ return ret;
|
|
}
|
|
|
|
static int validate_geometry_ddf_bvd(struct supertype *st,
|
|
@@ -3483,35 +3509,42 @@ static int validate_geometry_ddf_bvd(struct supertype *st,
|
|
static int load_super_ddf_all(struct supertype *st, int fd,
|
|
void **sbp, char *devname)
|
|
{
|
|
- struct mdinfo *sra;
|
|
- struct ddf_super *super;
|
|
struct mdinfo *sd, *best = NULL;
|
|
+ struct ddf_super *super = NULL;
|
|
+ struct mdinfo *sra;
|
|
int bestseq = 0;
|
|
- int seq;
|
|
+ int ret = 1;
|
|
char nm[20];
|
|
+ int seq;
|
|
int dfd;
|
|
|
|
sra = sysfs_read(fd, NULL, GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE);
|
|
if (!sra)
|
|
return 1;
|
|
- if (sra->array.major_version != -1 ||
|
|
- sra->array.minor_version != -2 ||
|
|
+ if (sra->array.major_version != -1 || sra->array.minor_version != -2 ||
|
|
strcmp(sra->text_version, "ddf") != 0)
|
|
- return 1;
|
|
+ goto out;
|
|
|
|
if (posix_memalign((void**)&super, 512, sizeof(*super)) != 0)
|
|
- return 1;
|
|
+ goto out;
|
|
+
|
|
memset(super, 0, sizeof(*super));
|
|
|
|
/* first, try each device, and choose the best ddf */
|
|
for (sd = sra->devs ; sd ; sd = sd->next) {
|
|
int rv;
|
|
+
|
|
sprintf(nm, "%d:%d", sd->disk.major, sd->disk.minor);
|
|
+
|
|
dfd = dev_open(nm, O_RDONLY);
|
|
- if (dfd < 0)
|
|
- return 2;
|
|
+ if (!is_fd_valid(dfd)) {
|
|
+ ret = 2;
|
|
+ goto out;
|
|
+ }
|
|
+
|
|
rv = load_ddf_headers(dfd, super, NULL);
|
|
close(dfd);
|
|
+
|
|
if (rv == 0) {
|
|
seq = be32_to_cpu(super->active->seq);
|
|
if (super->active->openflag)
|
|
@@ -3523,28 +3556,39 @@ static int load_super_ddf_all(struct supertype *st, int fd,
|
|
}
|
|
}
|
|
if (!best)
|
|
- return 1;
|
|
+ goto out;
|
|
+
|
|
/* OK, load this ddf */
|
|
sprintf(nm, "%d:%d", best->disk.major, best->disk.minor);
|
|
+
|
|
dfd = dev_open(nm, O_RDONLY);
|
|
if (dfd < 0)
|
|
- return 1;
|
|
+ goto out;
|
|
+
|
|
load_ddf_headers(dfd, super, NULL);
|
|
load_ddf_global(dfd, super, NULL);
|
|
close(dfd);
|
|
+
|
|
/* Now we need the device-local bits */
|
|
for (sd = sra->devs ; sd ; sd = sd->next) {
|
|
int rv;
|
|
|
|
sprintf(nm, "%d:%d", sd->disk.major, sd->disk.minor);
|
|
+
|
|
dfd = dev_open(nm, O_RDWR);
|
|
- if (dfd < 0)
|
|
- return 2;
|
|
+ if (dfd < 0) {
|
|
+ ret = 2;
|
|
+ goto out;
|
|
+ }
|
|
+
|
|
rv = load_ddf_headers(dfd, super, NULL);
|
|
if (rv == 0)
|
|
rv = load_ddf_local(dfd, super, NULL, 1);
|
|
- if (rv)
|
|
- return 1;
|
|
+
|
|
+ if (rv) {
|
|
+ close(dfd);
|
|
+ goto out;
|
|
+ }
|
|
}
|
|
|
|
*sbp = super;
|
|
@@ -3553,8 +3597,16 @@ static int load_super_ddf_all(struct supertype *st, int fd,
|
|
st->minor_version = 0;
|
|
st->max_devs = 512;
|
|
}
|
|
- strcpy(st->container_devnm, fd2devnm(fd));
|
|
- return 0;
|
|
+
|
|
+ snprintf(st->container_devnm, sizeof(st->container_devnm), "%s", fd2devnm(fd));
|
|
+ ret = 0;
|
|
+
|
|
+out:
|
|
+ if (sra)
|
|
+ free(sra);
|
|
+ if (super && ret != 0)
|
|
+ free(super);
|
|
+ return ret;
|
|
}
|
|
|
|
static int load_container_ddf(struct supertype *st, int fd,
|
|
@@ -3791,7 +3843,7 @@ static struct mdinfo *container_content_ddf(struct supertype *st, char *subarray
|
|
be64_to_cpu(LBA_OFFSET(ddf, bvd)[iphys]);
|
|
dev->component_size = be64_to_cpu(bvd->blocks);
|
|
if (d->devname)
|
|
- strcpy(dev->name, d->devname);
|
|
+ snprintf(dev->name, sizeof(dev->name), "%s", d->devname);
|
|
}
|
|
}
|
|
return rest;
|
|
@@ -3840,11 +3892,15 @@ static int store_super_ddf(struct supertype *st, int fd)
|
|
return 1;
|
|
memset(buf, 0, 512);
|
|
|
|
- lseek64(fd, dsize-512, 0);
|
|
+ if (lseek64(fd, dsize - 512, 0) == -1L) {
|
|
+ free(buf);
|
|
+ return 1;
|
|
+ }
|
|
rc = write(fd, buf, 512);
|
|
free(buf);
|
|
if (rc < 0)
|
|
return 1;
|
|
+
|
|
return 0;
|
|
}
|
|
|
|
@@ -3959,6 +4015,7 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst,
|
|
if (posix_memalign((void **)&dl1->spare, 512,
|
|
first->conf_rec_len*512) != 0) {
|
|
pr_err("could not allocate spare info buf\n");
|
|
+ free(dl1);
|
|
return 3;
|
|
}
|
|
memcpy(dl1->spare, dl2->spare, first->conf_rec_len*512);
|
|
@@ -4180,6 +4237,7 @@ static int get_bvd_state(const struct ddf_super *ddf,
|
|
state = DDF_state_part_optimal;
|
|
break;
|
|
}
|
|
+ free(avail);
|
|
return state;
|
|
}
|
|
|
|
--
|
|
2.41.0
|
|
|