Dan Williams: Always assume SKIP_GONE_DEVS behaviour and kill the flag
Martin F. Krafft
madduck at alioth.debian.org
Sat Aug 28 18:44:27 UTC 2010
Module: mdadm
Branch: build
Commit: b526e52dc7cbdde98db9c9f8765be28ba6d71d78
URL: http://git.debian.org/?p=pkg-mdadm/mdadm.git;a=commit;h=b526e52dc7cbdde98db9c9f8765be28ba6d71d78
Author: Dan Williams <dan.j.williams at intel.com>
Date: Wed Jun 16 17:26:04 2010 -0700
Always assume SKIP_GONE_DEVS behaviour and kill the flag
...i.e. GET_DEVS == (GET_DEVS|SKIP_GONE_DEVS)
A null pointer dereference in Incremental.c can be triggered by
replugging a disk while the old name is in use. When mdadm -I is called
on the new disk we fail the call to sysfs_read(). I audited all the
locations that use GET_DEVS and it appears they can tolerate missing a
drive. So just make SKIP_GONE_DEVS the default behaviour.
Also fix up remaining unchecked usages of the sysfs_read() return value.
Reported-by: Dave Jiang <dave.jiang at intel.com>
Signed-off-by: Dan Williams <dan.j.williams at intel.com>
---
Grow.c | 20 +++++++++++++-------
Incremental.c | 5 +++++
managemon.c | 2 +-
mapfile.c | 5 +++--
mdadm.h | 1 -
mdmon.c | 3 +--
super-ddf.c | 8 +-------
super-intel.c | 7 +------
sysfs.c | 20 +++++++++-----------
9 files changed, 34 insertions(+), 37 deletions(-)
diff --git a/Grow.c b/Grow.c
index 28ed8d7..3923a90 100644
--- a/Grow.c
+++ b/Grow.c
@@ -546,7 +546,13 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
return 1;
}
sra = sysfs_read(fd, 0, GET_LEVEL);
- frozen = freeze_array(sra);
+ if (sra)
+ frozen = freeze_array(sra);
+ else {
+ fprintf(stderr, Name ": failed to read sysfs parameters for %s\n",
+ devname);
+ return 1;
+ }
if (frozen < 0) {
fprintf(stderr, Name ": %s is performing resync/recovery and cannot"
" be reshaped\n", devname);
@@ -1970,6 +1976,12 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
int cache;
int done = 0;
+ sra = sysfs_read(-1, devname2devnum(info->sys_name),
+ GET_COMPONENT|GET_DEVS|GET_OFFSET|GET_STATE|
+ GET_CACHE);
+ if (!sra)
+ return 1;
+
err = sysfs_set_str(info, NULL, "array_state", "readonly");
if (err)
return err;
@@ -1990,7 +2002,6 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
ochunk = info->array.chunk_size;
nchunk = info->new_chunk;
-
a = (ochunk/512) * odata;
b = (nchunk/512) * ndata;
/* Find GCD */
@@ -2003,11 +2014,6 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
/* LCM == product / GCD */
blocks = (ochunk/512) * (nchunk/512) * odata * ndata / a;
- sra = sysfs_read(-1, devname2devnum(info->sys_name),
- GET_COMPONENT|GET_DEVS|GET_OFFSET|GET_STATE|
- GET_CACHE);
-
-
if (ndata == odata)
while (blocks * 32 < sra->component_size &&
blocks < 16*1024*2)
diff --git a/Incremental.c b/Incremental.c
index d6dd0f4..8062e2b 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -369,6 +369,8 @@ int Incremental(char *devname, int verbose, int runstop,
strcpy(chosen_name, devnum2devname(mp->devnum));
sra = sysfs_read(mdfd, fd2devnum(mdfd), (GET_DEVS | GET_STATE));
+ if (!sra)
+ return 2;
if (sra->devs) {
sprintf(dn, "%d:%d", sra->devs->disk.major,
@@ -586,6 +588,9 @@ static int count_active(struct supertype *st, int mdfd, char **availp,
struct mdinfo *sra = sysfs_read(mdfd, -1, GET_DEVS | GET_STATE);
char *avail = NULL;
+ if (!sra)
+ return 0;
+
for (d = sra->devs ; d ; d = d->next) {
char dn[30];
int dfd;
diff --git a/managemon.c b/managemon.c
index 454c39d..debca97 100644
--- a/managemon.c
+++ b/managemon.c
@@ -315,7 +315,7 @@ static void manage_container(struct mdstat_ent *mdstat,
* To see what is removed and what is added.
* These need to be remove from, or added to, the array
*/
- mdi = sysfs_read(-1, mdstat->devnum, GET_DEVS|SKIP_GONE_DEVS);
+ mdi = sysfs_read(-1, mdstat->devnum, GET_DEVS);
if (!mdi) {
/* invalidate the current count so we can try again */
container->devcnt = -1;
diff --git a/mapfile.c b/mapfile.c
index 0f12559..ffe8e16 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -368,7 +368,7 @@ void RebuildMap(void)
}
for (md = mdstat ; md ; md = md->next) {
- struct mdinfo *sra = sysfs_read(-1, md->devnum, GET_DEVS|SKIP_GONE_DEVS);
+ struct mdinfo *sra = sysfs_read(-1, md->devnum, GET_DEVS);
struct mdinfo *sd;
if (!sra)
@@ -486,7 +486,8 @@ void RebuildMap(void)
for (md = mdstat ; md ; md = md->next) {
struct mdinfo *sra = sysfs_read(-1, md->devnum,
GET_VERSION);
- sysfs_uevent(sra, "change");
+ if (sra)
+ sysfs_uevent(sra, "change");
sysfs_free(sra);
}
map_free(map);
diff --git a/mdadm.h b/mdadm.h
index a0797e8..62bfc44 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -404,7 +404,6 @@ enum sysfs_read_flags {
GET_SIZE = (1 << 12),
GET_STATE = (1 << 13),
GET_ERROR = (1 << 14),
- SKIP_GONE_DEVS = (1 << 15),
};
/* If fd >= 0, get the array it is open on,
diff --git a/mdmon.c b/mdmon.c
index 69c320e..2191814 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -394,8 +394,7 @@ static int mdmon(char *devname, int devnum, int must_fork, int takeover)
exit(3);
}
- mdi = sysfs_read(mdfd, container->devnum,
- GET_VERSION|GET_LEVEL|GET_DEVS|SKIP_GONE_DEVS);
+ mdi = sysfs_read(mdfd, container->devnum, GET_VERSION|GET_LEVEL|GET_DEVS);
if (!mdi) {
fprintf(stderr, "mdmon: failed to load sysfs info for %s\n",
diff --git a/super-ddf.c b/super-ddf.c
index b01c68d..6145e3c 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2807,14 +2807,8 @@ static int load_super_ddf_all(struct supertype *st, int fd,
int seq;
char nm[20];
int dfd;
- int devnum = fd2devnum(fd);
- enum sysfs_read_flags flags;
- flags = GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE;
- if (mdmon_running(devnum))
- flags |= SKIP_GONE_DEVS;
-
- sra = sysfs_read(fd, 0, flags);
+ sra = sysfs_read(fd, 0, GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE);
if (!sra)
return 1;
if (sra->array.major_version != -1 ||
diff --git a/super-intel.c b/super-intel.c
index d6d8b09..e09ce5e 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2747,14 +2747,9 @@ static int load_super_imsm_all(struct supertype *st, int fd, void **sbp,
int retry;
int err = 0;
int i;
- enum sysfs_read_flags flags;
-
- flags = GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE;
- if (mdmon_running(devnum))
- flags |= SKIP_GONE_DEVS;
/* check if 'fd' an opened container */
- sra = sysfs_read(fd, 0, flags);
+ sra = sysfs_read(fd, 0, GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE);
if (!sra)
return 1;
diff --git a/sysfs.c b/sysfs.c
index ebf9d8a..17f2567 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -273,22 +273,20 @@ struct mdinfo *sysfs_read(int fd, int devnum, unsigned long options)
strcpy(dbase, "block/dev");
if (load_sys(fname, buf)) {
+ /* assume this is a stale reference to a hot
+ * removed device
+ */
free(dev);
- if (options & SKIP_GONE_DEVS)
- continue;
- else
- goto abort;
+ continue;
}
sscanf(buf, "%d:%d", &dev->disk.major, &dev->disk.minor);
/* special case check for block devices that can go 'offline' */
- if (options & SKIP_GONE_DEVS) {
- strcpy(dbase, "block/device/state");
- if (load_sys(fname, buf) == 0 &&
- strncmp(buf, "offline", 7) == 0) {
- free(dev);
- continue;
- }
+ strcpy(dbase, "block/device/state");
+ if (load_sys(fname, buf) == 0 &&
+ strncmp(buf, "offline", 7) == 0) {
+ free(dev);
+ continue;
}
/* finally add this disk to the array */
More information about the pkg-mdadm-commits
mailing list