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@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
This commit is contained in:
Dan Williams 2010-06-16 17:26:04 -07:00
parent fd547b508c
commit b526e52dc7
9 changed files with 34 additions and 37 deletions

20
Grow.c
View File

@ -546,7 +546,13 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
return 1; return 1;
} }
sra = sysfs_read(fd, 0, GET_LEVEL); 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) { if (frozen < 0) {
fprintf(stderr, Name ": %s is performing resync/recovery and cannot" fprintf(stderr, Name ": %s is performing resync/recovery and cannot"
" be reshaped\n", devname); " be reshaped\n", devname);
@ -1970,6 +1976,12 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
int cache; int cache;
int done = 0; 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"); err = sysfs_set_str(info, NULL, "array_state", "readonly");
if (err) if (err)
return err; return err;
@ -1990,7 +2002,6 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
ochunk = info->array.chunk_size; ochunk = info->array.chunk_size;
nchunk = info->new_chunk; nchunk = info->new_chunk;
a = (ochunk/512) * odata; a = (ochunk/512) * odata;
b = (nchunk/512) * ndata; b = (nchunk/512) * ndata;
/* Find GCD */ /* Find GCD */
@ -2003,11 +2014,6 @@ int Grow_continue(int mdfd, struct supertype *st, struct mdinfo *info,
/* LCM == product / GCD */ /* LCM == product / GCD */
blocks = (ochunk/512) * (nchunk/512) * odata * ndata / a; 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) if (ndata == odata)
while (blocks * 32 < sra->component_size && while (blocks * 32 < sra->component_size &&
blocks < 16*1024*2) blocks < 16*1024*2)

View File

@ -369,6 +369,8 @@ int Incremental(char *devname, int verbose, int runstop,
strcpy(chosen_name, devnum2devname(mp->devnum)); strcpy(chosen_name, devnum2devname(mp->devnum));
sra = sysfs_read(mdfd, fd2devnum(mdfd), (GET_DEVS | GET_STATE)); sra = sysfs_read(mdfd, fd2devnum(mdfd), (GET_DEVS | GET_STATE));
if (!sra)
return 2;
if (sra->devs) { if (sra->devs) {
sprintf(dn, "%d:%d", sra->devs->disk.major, 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); struct mdinfo *sra = sysfs_read(mdfd, -1, GET_DEVS | GET_STATE);
char *avail = NULL; char *avail = NULL;
if (!sra)
return 0;
for (d = sra->devs ; d ; d = d->next) { for (d = sra->devs ; d ; d = d->next) {
char dn[30]; char dn[30];
int dfd; int dfd;

View File

@ -315,7 +315,7 @@ static void manage_container(struct mdstat_ent *mdstat,
* To see what is removed and what is added. * To see what is removed and what is added.
* These need to be remove from, or added to, the array * 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) { if (!mdi) {
/* invalidate the current count so we can try again */ /* invalidate the current count so we can try again */
container->devcnt = -1; container->devcnt = -1;

View File

@ -368,7 +368,7 @@ void RebuildMap(void)
} }
for (md = mdstat ; md ; md = md->next) { 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; struct mdinfo *sd;
if (!sra) if (!sra)
@ -486,7 +486,8 @@ void RebuildMap(void)
for (md = mdstat ; md ; md = md->next) { for (md = mdstat ; md ; md = md->next) {
struct mdinfo *sra = sysfs_read(-1, md->devnum, struct mdinfo *sra = sysfs_read(-1, md->devnum,
GET_VERSION); GET_VERSION);
sysfs_uevent(sra, "change"); if (sra)
sysfs_uevent(sra, "change");
sysfs_free(sra); sysfs_free(sra);
} }
map_free(map); map_free(map);

View File

@ -404,7 +404,6 @@ enum sysfs_read_flags {
GET_SIZE = (1 << 12), GET_SIZE = (1 << 12),
GET_STATE = (1 << 13), GET_STATE = (1 << 13),
GET_ERROR = (1 << 14), GET_ERROR = (1 << 14),
SKIP_GONE_DEVS = (1 << 15),
}; };
/* If fd >= 0, get the array it is open on, /* If fd >= 0, get the array it is open on,

View File

@ -394,8 +394,7 @@ static int mdmon(char *devname, int devnum, int must_fork, int takeover)
exit(3); exit(3);
} }
mdi = sysfs_read(mdfd, container->devnum, mdi = sysfs_read(mdfd, container->devnum, GET_VERSION|GET_LEVEL|GET_DEVS);
GET_VERSION|GET_LEVEL|GET_DEVS|SKIP_GONE_DEVS);
if (!mdi) { if (!mdi) {
fprintf(stderr, "mdmon: failed to load sysfs info for %s\n", fprintf(stderr, "mdmon: failed to load sysfs info for %s\n",

View File

@ -2807,14 +2807,8 @@ static int load_super_ddf_all(struct supertype *st, int fd,
int seq; int seq;
char nm[20]; char nm[20];
int dfd; int dfd;
int devnum = fd2devnum(fd);
enum sysfs_read_flags flags;
flags = GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE; sra = sysfs_read(fd, 0, GET_LEVEL|GET_VERSION|GET_DEVS|GET_STATE);
if (mdmon_running(devnum))
flags |= SKIP_GONE_DEVS;
sra = sysfs_read(fd, 0, flags);
if (!sra) if (!sra)
return 1; return 1;
if (sra->array.major_version != -1 || if (sra->array.major_version != -1 ||

View File

@ -2747,14 +2747,9 @@ static int load_super_imsm_all(struct supertype *st, int fd, void **sbp,
int retry; int retry;
int err = 0; int err = 0;
int i; 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 */ /* 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) if (!sra)
return 1; return 1;

20
sysfs.c
View File

@ -273,22 +273,20 @@ struct mdinfo *sysfs_read(int fd, int devnum, unsigned long options)
strcpy(dbase, "block/dev"); strcpy(dbase, "block/dev");
if (load_sys(fname, buf)) { if (load_sys(fname, buf)) {
/* assume this is a stale reference to a hot
* removed device
*/
free(dev); free(dev);
if (options & SKIP_GONE_DEVS) continue;
continue;
else
goto abort;
} }
sscanf(buf, "%d:%d", &dev->disk.major, &dev->disk.minor); sscanf(buf, "%d:%d", &dev->disk.major, &dev->disk.minor);
/* special case check for block devices that can go 'offline' */ /* special case check for block devices that can go 'offline' */
if (options & SKIP_GONE_DEVS) { strcpy(dbase, "block/device/state");
strcpy(dbase, "block/device/state"); if (load_sys(fname, buf) == 0 &&
if (load_sys(fname, buf) == 0 && strncmp(buf, "offline", 7) == 0) {
strncmp(buf, "offline", 7) == 0) { free(dev);
free(dev); continue;
continue;
}
} }
/* finally add this disk to the array */ /* finally add this disk to the array */