Module Name: src Committed By: kre Date: Wed Nov 22 00:31:31 UTC 2017
Modified Files: src/sbin/raidctl: rf_configure.c Log Message: Several more cleanups: 1. Don't force use of "for" when "while" works better. 2. No need to check c != '\0' when we also check (c == ' ' || c == '\t') 3. Use the size of the buffer we're using, rather than a different one (not really a concern, they're the same size) 4. Don't use fscanf() to read file data, use fgets() & sscanf(). 5. After using a pointer as a char *, validate alignment before switching to int * (can only fail if kernel #define gets set stupidly) Or #6... 6. Validate sparemap file name isn't too long for assigned space. 7. recognise that strlen() returns size_t - don't shove it into an int. 8. On out of mem, be more clear which allocation failed in warning msg. ATF tests all pass. But I don't think they use sparemap files. To generate a diff of this commit: cvs rdiff -u -r1.31 -r1.32 src/sbin/raidctl/rf_configure.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sbin/raidctl/rf_configure.c diff -u src/sbin/raidctl/rf_configure.c:1.31 src/sbin/raidctl/rf_configure.c:1.32 --- src/sbin/raidctl/rf_configure.c:1.31 Tue Nov 21 16:31:37 2017 +++ src/sbin/raidctl/rf_configure.c Wed Nov 22 00:31:31 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: rf_configure.c,v 1.31 2017/11/21 16:31:37 christos Exp $ */ +/* $NetBSD: rf_configure.c,v 1.32 2017/11/22 00:31:31 kre Exp $ */ /* * Copyright (c) 1995 Carnegie-Mellon University. @@ -49,7 +49,7 @@ #include <sys/cdefs.h> #ifndef lint -__RCSID("$NetBSD: rf_configure.c,v 1.31 2017/11/21 16:31:37 christos Exp $"); +__RCSID("$NetBSD: rf_configure.c,v 1.32 2017/11/22 00:31:31 kre Exp $"); #endif @@ -384,12 +384,18 @@ rf_MakeLayoutSpecificDeclustered(FILE *c * daemon that's responsible for finding the sparemaps */ if (distSpare) { - if (rf_get_next_nonblank_line(smbuf, sizeof(buf), configfp, + if (rf_get_next_nonblank_line(smbuf, sizeof(smbuf), configfp, "Can't find sparemap file name in config file")) { fclose(fp); return EINVAL; } smname = rf_find_non_white(smbuf, 1); + if (strlen(smname) >= RF_SPAREMAP_NAME_LEN) { + warnx("sparemap file name '%s' too long (max %d)", + smname, RF_SPAREMAP_NAME_LEN - 1); + fclose(fp); + return EINVAL; + } } else { smbuf[0] = '\0'; smname = smbuf; @@ -415,6 +421,13 @@ rf_MakeLayoutSpecificDeclustered(FILE *c *p++ = '\0'; i++; } + if ((i & (sizeof(int) - 1)) != 0) { + /* panic, unaligned data; RF_SPAREMAP_NAME_LEN invalid */ + warnx("Program Bug: (RF_SPAREMAP_NAME_LEN(%d) %% %zd) != 0", + RF_SPAREMAP_NAME_LEN, sizeof(int)); + fclose(fp); + return EINVAL; + } /* * fill in the buffer with the block design parameters @@ -454,8 +467,8 @@ rf_MakeLayoutSpecificDeclustered(FILE *c static char * rf_find_non_white(char *p, int eatnl) { - for (; *p != '\0' && (*p == ' ' || *p == '\t'); p++) - continue; + while (*p == ' ' || *p == '\t') + p++; if (*p == '\n' && eatnl) *p = '\0'; return p; @@ -465,8 +478,8 @@ rf_find_non_white(char *p, int eatnl) static char * rf_find_white(char *p) { - for (; *p != '\0' && *p != ' ' && *p != '\t'; p++) - continue; + while (*p != '\0' && *p != ' ' && *p != '\t') + p++; return p; } @@ -497,17 +510,15 @@ static int rf_get_next_nonblank_line(char *buf, int len, FILE *fp, const char *errmsg) { char *p; - int l; + size_t l; while (fgets(buf, len, fp) != NULL) { p = rf_find_non_white(buf, 0); if (*p == '\n' || *p == '\0' || *p == '#') continue; - l = strlen(buf) - 1; - while (l >= 0 && (buf[l] == ' ' || buf[l] == '\n')) { + l = strlen(buf); + while (l > 0 && (buf[--l] == ' ' || buf[l] == '\n')) buf[l] = '\0'; - l--; - } return 0; } if (errmsg) @@ -536,6 +547,7 @@ rf_ReadSpareTable(RF_SparetWait_t *req, char buf[BUFSIZ], targString[BUFSIZ], errString[BUFSIZ]; RF_SpareTableEntry_t **table; FILE *fp = NULL; + size_t len; /* allocate and initialize the table */ table = calloc(req->TablesPerSpareRegion, sizeof(*table)); @@ -546,7 +558,7 @@ rf_ReadSpareTable(RF_SparetWait_t *req, for (i = 0; i < req->TablesPerSpareRegion; i++) { table[i] = calloc(req->BlocksPerTable, sizeof(**table)); if (table[i] == NULL) { - warn("%s: Unable to allocate table", __func__); + warn("%s: Unable to allocate table:%d", __func__, i); goto out; } for (j = 0; j < req->BlocksPerTable; j++) @@ -563,7 +575,7 @@ rf_ReadSpareTable(RF_SparetWait_t *req, "Invalid sparemap file: can't find header line")) goto out; - size_t len = strlen(buf); + len = strlen(buf); if (len != 0 && buf[len - 1] == '\n') buf[len - 1] = '\0'; @@ -579,13 +591,21 @@ rf_ReadSpareTable(RF_SparetWait_t *req, /* no more blank lines or comments allowed now */ linecount = req->TablesPerSpareRegion * req->TableDepthInPUs; for (i = 0; i < linecount; i++) { - numFound = fscanf(fp, " %d %d %d %d", &tableNum, &tupleNum, - &spareDisk, &spareBlkOffset); - if (numFound != 4) { + char linebuf[BUFSIZ]; + + if (fgets(linebuf, BUFSIZ, fp) == NULL) { warnx("Sparemap file prematurely exhausted after %d " "of %d lines", i, linecount); goto out; } + numFound = sscanf(linebuf, " %d %d %d %d", &tableNum, &tupleNum, + &spareDisk, &spareBlkOffset); + if (numFound != 4) { + warnx("Sparemap file format error - " + "line %d of %d lines", + i + 1, linecount); + goto out; + } table[tableNum][tupleNum].spareDisk = spareDisk; table[tableNum][tupleNum].spareBlockOffsetInSUs =