Martchus
/
mdadm
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

load_sys(): Add a buffer size argument 

This adds a buffer size argument to load_sys(), rather than relying on
a hard coded buffer size. The old behavior was safe because we knew
the kernel would never return strings overrunning the buffers, however
it was ugly, and would cause code checking tools to spit out warnings.

This caused a Coverity warning over the read into
sra->sysfs_array_state which is only 20 bytes.

Reviewed-by: NeilBrown <neilb@suse.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
This commit is contained in:
Jes Sorensen 2016-03-04 16:00:21 -05:00
parent 2a1990c0f4
commit 193b6c0b26
4 changed files with 29 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Detail.c
View File

@ -582,7 +582,7 @@ This is pretty boring
continue; continue;
sprintf(path, "/sys/block/%s/md/metadata_version", sprintf(path, "/sys/block/%s/md/metadata_version",
de->d_name); de->d_name);
if (load_sys(path, vbuf) < 0) if (load_sys(path, vbuf, sizeof(vbuf)) < 0)
continue; continue;
if (strncmp(vbuf, "external:", 9) != 0 || if (strncmp(vbuf, "external:", 9) != 0 ||
!is_subarray(vbuf+9) || !is_subarray(vbuf+9) ||

2
mdadm.h
View File

@ -631,7 +631,7 @@ extern int sysfs_disk_to_scsi_id(int fd, __u32 *id);
extern int sysfs_unique_holder(char *devnm, long rdev); extern int sysfs_unique_holder(char *devnm, long rdev);
extern int sysfs_freeze_array(struct mdinfo *sra); extern int sysfs_freeze_array(struct mdinfo *sra);
extern int sysfs_wait(int fd, int *msec); extern int sysfs_wait(int fd, int *msec);
extern int load_sys(char *path, char *buf); extern int load_sys(char *path, char *buf, int len);
extern int reshape_prepare_fdlist(char *devname, extern int reshape_prepare_fdlist(char *devname,
struct mdinfo *sra, struct mdinfo *sra,
int raid_disks, int raid_disks,

6
super-intel.c
View File

@ -1654,7 +1654,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
break; break;
} }
sprintf(device, "/sys/dev/block/%d:%d/device/type", major, minor); sprintf(device, "/sys/dev/block/%d:%d/device/type", major, minor);
if (load_sys(device, buf) != 0) { if (load_sys(device, buf, sizeof(buf)) != 0) {
if (verbose > 0) if (verbose > 0)
pr_err("failed to read device type for %s\n", pr_err("failed to read device type for %s\n",
path); path);
@ -1669,7 +1669,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
vendor[0] = '\0'; vendor[0] = '\0';
model[0] = '\0'; model[0] = '\0';
sprintf(device, "/sys/dev/block/%d:%d/device/vendor", major, minor); sprintf(device, "/sys/dev/block/%d:%d/device/vendor", major, minor);
if (load_sys(device, buf) == 0) { if (load_sys(device, buf, sizeof(buf)) == 0) {
strncpy(vendor, buf, sizeof(vendor)); strncpy(vendor, buf, sizeof(vendor));
vendor[sizeof(vendor) - 1] = '\0'; vendor[sizeof(vendor) - 1] = '\0';
c = (char *) &vendor[sizeof(vendor) - 1]; c = (char *) &vendor[sizeof(vendor) - 1];
@ -1678,7 +1678,7 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
} }
sprintf(device, "/sys/dev/block/%d:%d/device/model", major, minor); sprintf(device, "/sys/dev/block/%d:%d/device/model", major, minor);
if (load_sys(device, buf) == 0) { if (load_sys(device, buf, sizeof(buf)) == 0) {
strncpy(model, buf, sizeof(model)); strncpy(model, buf, sizeof(model));
model[sizeof(model) - 1] = '\0'; model[sizeof(model) - 1] = '\0';
c = (char *) &model[sizeof(model) - 1]; c = (char *) &model[sizeof(model) - 1];

47
sysfs.c
View File

@ -27,15 +27,15 @@
#include <dirent.h> #include <dirent.h>
#include <ctype.h> #include <ctype.h>
int load_sys(char *path, char *buf) int load_sys(char *path, char *buf, int len)
{ {
int fd = open(path, O_RDONLY); int fd = open(path, O_RDONLY);
int n; int n;
if (fd < 0) if (fd < 0)
return -1; return -1;
n = read(fd, buf, 1024); n = read(fd, buf, len);
close(fd); close(fd);
if (n <0 || n >= 1024) if (n <0 || n >= len)
return -1; return -1;
buf[n] = 0; buf[n] = 0;
if (n && buf[n-1] == '\n') if (n && buf[n-1] == '\n')
@ -118,7 +118,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
sra->devs = NULL; sra->devs = NULL;
if (options & GET_VERSION) { if (options & GET_VERSION) {
strcpy(base, "metadata_version"); strcpy(base, "metadata_version");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
if (strncmp(buf, "none", 4) == 0) { if (strncmp(buf, "none", 4) == 0) {
sra->array.major_version = sra->array.major_version =
@ -137,31 +137,31 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
} }
if (options & GET_LEVEL) { if (options & GET_LEVEL) {
strcpy(base, "level"); strcpy(base, "level");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->array.level = map_name(pers, buf); sra->array.level = map_name(pers, buf);
} }
if (options & GET_LAYOUT) { if (options & GET_LAYOUT) {
strcpy(base, "layout"); strcpy(base, "layout");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->array.layout = strtoul(buf, NULL, 0); sra->array.layout = strtoul(buf, NULL, 0);
} }
if (options & GET_DISKS) { if (options & GET_DISKS) {
strcpy(base, "raid_disks"); strcpy(base, "raid_disks");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->array.raid_disks = strtoul(buf, NULL, 0); sra->array.raid_disks = strtoul(buf, NULL, 0);
} }
if (options & GET_DEGRADED) { if (options & GET_DEGRADED) {
strcpy(base, "degraded"); strcpy(base, "degraded");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->array.failed_disks = strtoul(buf, NULL, 0); sra->array.failed_disks = strtoul(buf, NULL, 0);
} }
if (options & GET_COMPONENT) { if (options & GET_COMPONENT) {
strcpy(base, "component_size"); strcpy(base, "component_size");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->component_size = strtoull(buf, NULL, 0); sra->component_size = strtoull(buf, NULL, 0);
/* sysfs reports "K", but we want sectors */ /* sysfs reports "K", but we want sectors */
@ -169,13 +169,13 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
} }
if (options & GET_CHUNK) { if (options & GET_CHUNK) {
strcpy(base, "chunk_size"); strcpy(base, "chunk_size");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->array.chunk_size = strtoul(buf, NULL, 0); sra->array.chunk_size = strtoul(buf, NULL, 0);
} }
if (options & GET_CACHE) { if (options & GET_CACHE) {
strcpy(base, "stripe_cache_size"); strcpy(base, "stripe_cache_size");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
/* Probably level doesn't support it */ /* Probably level doesn't support it */
sra->cache_size = 0; sra->cache_size = 0;
else else
@ -183,7 +183,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
} }
if (options & GET_MISMATCH) { if (options & GET_MISMATCH) {
strcpy(base, "mismatch_cnt"); strcpy(base, "mismatch_cnt");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
sra->mismatch_cnt = strtoul(buf, NULL, 0); sra->mismatch_cnt = strtoul(buf, NULL, 0);
} }
@ -195,7 +195,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
size_t len; size_t len;
strcpy(base, "safe_mode_delay"); strcpy(base, "safe_mode_delay");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
/* remove a period, and count digits after it */ /* remove a period, and count digits after it */
@ -218,7 +218,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
} }
if (options & GET_BITMAP_LOCATION) { if (options & GET_BITMAP_LOCATION) {
strcpy(base, "bitmap/location"); strcpy(base, "bitmap/location");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
if (strncmp(buf, "file", 4) == 0) if (strncmp(buf, "file", 4) == 0)
sra->bitmap_offset = 1; sra->bitmap_offset = 1;
@ -232,7 +232,8 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (options & GET_ARRAY_STATE) { if (options & GET_ARRAY_STATE) {
strcpy(base, "array_state"); strcpy(base, "array_state");
if (load_sys(fname, sra->sysfs_array_state)) if (load_sys(fname, sra->sysfs_array_state,
sizeof(sra->sysfs_array_state)))
goto abort; goto abort;
} else } else
sra->sysfs_array_state[0] = 0; sra->sysfs_array_state[0] = 0;
@ -262,7 +263,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
/* Always get slot, major, minor */ /* Always get slot, major, minor */
strcpy(dbase, "slot"); strcpy(dbase, "slot");
if (load_sys(fname, buf)) { if (load_sys(fname, buf, sizeof(buf))) {
/* hmm... unable to read 'slot' maybe the device /* hmm... unable to read 'slot' maybe the device
* is going away? * is going away?
*/ */
@ -287,7 +288,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (*ep) dev->disk.raid_disk = -1; if (*ep) dev->disk.raid_disk = -1;
strcpy(dbase, "block/dev"); strcpy(dbase, "block/dev");
if (load_sys(fname, buf)) { if (load_sys(fname, buf, sizeof(buf))) {
/* assume this is a stale reference to a hot /* assume this is a stale reference to a hot
* removed device * removed device
*/ */
@ -299,7 +300,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
/* special case check for block devices that can go 'offline' */ /* special case check for block devices that can go 'offline' */
strcpy(dbase, "block/device/state"); strcpy(dbase, "block/device/state");
if (load_sys(fname, buf) == 0 && if (load_sys(fname, buf, sizeof(buf)) == 0 &&
strncmp(buf, "offline", 7) == 0) { strncmp(buf, "offline", 7) == 0) {
free(dev); free(dev);
continue; continue;
@ -312,25 +313,25 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
if (options & GET_OFFSET) { if (options & GET_OFFSET) {
strcpy(dbase, "offset"); strcpy(dbase, "offset");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
dev->data_offset = strtoull(buf, NULL, 0); dev->data_offset = strtoull(buf, NULL, 0);
strcpy(dbase, "new_offset"); strcpy(dbase, "new_offset");
if (load_sys(fname, buf) == 0) if (load_sys(fname, buf, sizeof(buf)) == 0)
dev->new_data_offset = strtoull(buf, NULL, 0); dev->new_data_offset = strtoull(buf, NULL, 0);
else else
dev->new_data_offset = dev->data_offset; dev->new_data_offset = dev->data_offset;
} }
if (options & GET_SIZE) { if (options & GET_SIZE) {
strcpy(dbase, "size"); strcpy(dbase, "size");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
dev->component_size = strtoull(buf, NULL, 0) * 2; dev->component_size = strtoull(buf, NULL, 0) * 2;
} }
if (options & GET_STATE) { if (options & GET_STATE) {
dev->disk.state = 0; dev->disk.state = 0;
strcpy(dbase, "state"); strcpy(dbase, "state");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
if (strstr(buf, "in_sync")) if (strstr(buf, "in_sync"))
dev->disk.state |= (1<<MD_DISK_SYNC); dev->disk.state |= (1<<MD_DISK_SYNC);
@ -341,7 +342,7 @@ struct mdinfo *sysfs_read(int fd, char *devnm, unsigned long options)
} }
if (options & GET_ERROR) { if (options & GET_ERROR) {
strcpy(buf, "errors"); strcpy(buf, "errors");
if (load_sys(fname, buf)) if (load_sys(fname, buf, sizeof(buf)))
goto abort; goto abort;
dev->errors = strtoul(buf, NULL, 0); dev->errors = strtoul(buf, NULL, 0);
} }