summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDoug Ledford <dledford@redhat.com>2010-07-20 17:42:26 -0400
committerDoug Ledford <dledford@redhat.com>2010-07-22 10:16:31 -0400
commit56850593049e792c1775047481f814dd444042c5 (patch)
treeb849902282eabc1c8bd3e3284a365452a4a0290d
parent0155af90d8352d3ca031347e75854b3a5a4052ac (diff)
downloadmdadm-for-neil.zip
mdadm-for-neil.tar.gz
mdadm-for-neil.tar.xz
Bugfix: mapfile locking is broken/racyfor-neil
While we attempt to use a lockfile to grant exclusive access to the mapfile, our implementation is buggy. Specifically, we create a lockfile, then lock it, at which point new instances can open the lockfile and attempt to lock it, which will cause them to block. However, when we are ready to unlock it, we unlink the file. This causes existing lock waiters to get a lock on an unlinked inode while a different instance may now create a new lockfile and get an exclusive lock on it. There are two possible fixes: don't unlink the lock file or require that the open of the lockfile include the O_CREAT|O_EXCL flags and on EEXIST retry the open until we either fail for a different error or actually create the lock file. I implemented the later on the assumption that the md device map file and therefore the lockfile are on local, early read write storage (probably a tmpfs of some sort) and therefore not subject to the problem that it doesn't work over nfs2 or earlier. In addition add warnings if we ever fail to get a lock when we request it. Signed-off-by: Doug Ledford <dledford@redhat.com>
-rw-r--r--Incremental.c8
-rw-r--r--mapfile.c16
2 files changed, 11 insertions, 13 deletions
diff --git a/Incremental.c b/Incremental.c
index 35490e7..a8a072d 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -294,7 +294,9 @@ int Incremental(char *devname, int verbose, int runstop,
/* 4/ Check if array exists.
*/
- map_lock(&map);
+ if (map_lock(&map))
+ fprintf(stderr, Name ": failed to get exclusive lock on "
+ "mapfile\n");
mp = map_by_uuid(&map, info.uuid);
if (mp)
mdfd = open_dev(mp->devnum);
@@ -793,7 +795,9 @@ int Incremental_container(struct supertype *st, char *devname, int verbose,
struct mdinfo *ra;
struct map_ent *map = NULL;
- map_lock(&map);
+ if (map_lock(&map))
+ fprintf(stderr, Name ": failed to get exclusive lock on "
+ "mapfile\n");
for (ra = list ; ra ; ra = ra->next) {
int mdfd;
diff --git a/mapfile.c b/mapfile.c
index 79a6bd8..69f78b1 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -58,7 +58,7 @@ char *mapname[2][3] = {
};
char *mapdir[2] = { MAP_DIR, NULL };
-int mapmode[3] = { O_RDONLY, O_RDWR|O_CREAT, O_RDWR|O_CREAT|O_TRUNC };
+int mapmode[3] = { O_RDONLY, O_RDWR|O_CREAT, O_RDWR|O_CREAT|O_EXCL };
char *mapsmode[3] = { "r", "w", "w"};
FILE *open_map(int modenum, int *choice)
@@ -120,14 +120,11 @@ static int lwhich = 0;
int map_lock(struct map_ent **melp)
{
if (lf == NULL) {
- lf = open_map(2, &lwhich);
+ do {
+ lf = open_map(2, &lwhich);
+ } while (lf == NULL && errno == EEXIST);
if (lf == NULL)
return -1;
- if (flock(fileno(lf), LOCK_EX) != 0) {
- fclose(lf);
- lf = NULL;
- return -1;
- }
}
if (*melp)
map_free(*melp);
@@ -137,10 +134,7 @@ int map_lock(struct map_ent **melp)
void map_unlock(struct map_ent **melp)
{
- if (lf) {
- flock(fileno(lf), LOCK_UN);
- fclose(lf);
- }
+ fclose(lf);
unlink(mapname[lwhich][2]);
lf = NULL;
}