rpms/mdadm
7
0
Fork 0
Code Issues Pull Requests Releases Activity
c9s
mdadm/0116-mdadm-super-ddf.c-fix-coverity-issues.patch
Xiao Ni f1b7707f26 Update to latest upstream 
For issue RHEL-40729, patch 0142 fixes it.
For issue RHEL-31448, patch 0125 fixes it.

Resolves: RHEL-31448,RHEL-40729,RHEL-52059

Signed-off-by: Xiao Ni <xni@redhat.com>
2024-10-19 05:22:07 -04:00

474 lines
13 KiB
Diff
Raw Permalink Blame History

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/201] 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
Reference in New Issue View Git Blame Copy Permalink