summaryrefslogtreecommitdiff
path: root/mapfile.c
diff options
context:
space:
mode:
authorNeilBrown <neilb@suse.de>2010-07-28 17:40:54 +1000
committerNeilBrown <neilb@suse.de>2010-07-28 17:40:54 +1000
commit08415c46a867b26743b6b4fc93d5fc6ed027cb07 (patch)
tree3837b21f857b745b978f03642542b2502e3bda7e /mapfile.c
parent93c861ee5752896b340da51afb8ad4db95ae5e3b (diff)
downloadmdadm-08415c46a867b26743b6b4fc93d5fc6ed027cb07.tar.gz
Bugfix: mapfile locking is broken/racy
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 several possible fixes. The chosen one is to test if ->s_nlink is zero after we get the lock and to retry if it isn't. This means: - failing to unlink a file doesn't leave a stale lock - we can block waiting to get a lock rather than busy-waiting - we don't need to leave a lock file permanently in place. Reported-by: Doug Ledford <dledford@redhat.com> Signed-off-by: NeilBrown <neilb@suse.de>
Diffstat (limited to 'mapfile.c')
-rw-r--r--mapfile.c19
1 files changed, 16 insertions, 3 deletions
diff --git a/mapfile.c b/mapfile.c
index 79a6bd8..8e40b6b 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -119,7 +119,8 @@ static FILE *lf = NULL;
static int lwhich = 0;
int map_lock(struct map_ent **melp)
{
- if (lf == NULL) {
+ while (lf == NULL) {
+ struct stat buf;
lf = open_map(2, &lwhich);
if (lf == NULL)
return -1;
@@ -128,6 +129,15 @@ int map_lock(struct map_ent **melp)
lf = NULL;
return -1;
}
+ if (fstat(fileno(lf), &buf) != 0 ||
+ buf.st_nlink == 0) {
+ /* The owner of the lock unlinked it,
+ * so we have a lock on a stale file,
+ * try again
+ */
+ fclose(lf);
+ lf = NULL;
+ }
}
if (*melp)
map_free(*melp);
@@ -138,10 +148,13 @@ int map_lock(struct map_ent **melp)
void map_unlock(struct map_ent **melp)
{
if (lf) {
- flock(fileno(lf), LOCK_UN);
+ /* must unlink before closing the file,
+ * as only the owner of the lock may
+ * unlink the file
+ */
+ unlink(mapname[lwhich][2]);
fclose(lf);
}
- unlink(mapname[lwhich][2]);
lf = NULL;
}