diff options
| author | kre <kre@NetBSD.org> | 2017-11-22 00:31:31 +0000 |
|---|---|---|
| committer | kre <kre@NetBSD.org> | 2017-11-22 00:31:31 +0000 |
| commit | d6bf4e18248a90e33bf6d6fa9bdb9407e114fe2f (patch) | |
| tree | 333c2851d19866ae73c19ceb7e164de90d1503c1 /sbin/raidctl | |
| parent | 94e99d042ab66c20a4795cd5350ea7994c08ad8f (diff) | |
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.
Diffstat (limited to 'sbin/raidctl')
| -rw-r--r-- | sbin/raidctl/rf_configure.c | 54 |
1 files changed, 37 insertions, 17 deletions
diff --git a/sbin/raidctl/rf_configure.c b/sbin/raidctl/rf_configure.c index cd646d45cf0..4ee138ea7eb 100644 --- a/sbin/raidctl/rf_configure.c +++ b/sbin/raidctl/rf_configure.c @@ -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 *configfp, RF_Config_t *cfgPtr, void *arg) * 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 *configfp, RF_Config_t *cfgPtr, void *arg) *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 *configfp, RF_Config_t *cfgPtr, void *arg) 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 *fname) 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, char *fname) 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, char *fname) "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, char *fname) /* 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 = |
