From 96b8035a09b6449ea99f2eb91f9ba4f6912e5bd6 Mon Sep 17 00:00:00 2001 From: Nigel Croxon 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 --- 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