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 =
