mir/openvswitch
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-10-21 14:49:41 +00:00
Code Issues Releases Wiki Activity

lockfile: Fix hang locking through a dangling symlink.

Browse Source 
open() with O_CREAT|O_EXCL yields EEXIST if the file being opened is a
symlink.  lockfile_try_lock() interpreted that error code to mean that
some other process had created the lock file in the meantime, so it went
around its loop again, which found out the same thing, which led to a hang.

This commit fixes the problem by dropping O_EXCL.  I don't see any reason
that it's actually necessary.  That means that the loop itself is
unnecessary, so this commit drops that too.

Debian bug #681880.
CC: 681880@bugs.debian.org
Reported-by: Bastian Blank <waldi@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
This commit is contained in:
Ben Pfaff
2012-07-26 14:36:24 -07:00
parent b322930908
commit 77c513a44a
3 changed files with 54 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
lib/lockfile.c
View File

@@ -1,4 +1,4 @@
/* Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
/* Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -221,8 +221,6 @@ lockfile_try_lock(const char *name, bool block, struct lockfile **lockfilep)
*lockfilep = NULL;
/* Open the lock file, first creating it if necessary. */
for (;;) {
/* Check whether we've already got a lock on that file. */
if (!stat(name, &s)) {
if (lockfile_find(s.st_dev, s.st_ino)) {
@@ -234,30 +232,14 @@ lockfile_try_lock(const char *name, bool block, struct lockfile **lockfilep)
return errno;
}
/* Try to open an existing lock file. */
fd = open(name, O_RDWR);
if (fd >= 0) {
break;
} else if (errno != ENOENT) {
/* Open the lock file. */
fd = open(name, O_RDWR | O_CREAT, 0600);
if (fd < 0) {
VLOG_WARN("%s: failed to open lock file: %s",
name, strerror(errno));
return errno;
}
/* Try to create a new lock file. */
VLOG_INFO("%s: lock file does not exist, creating", name);
fd = open(name, O_RDWR | O_CREAT | O_EXCL, 0600);
if (fd >= 0) {
break;
} else if (errno != EEXIST) {
VLOG_WARN("%s: failed to create lock file: %s",
name, strerror(errno));
return errno;
}
/* Someone else created the lock file. Try again. */
}
/* Get the inode and device number for the lock table. */
if (fstat(fd, &s)) {
VLOG_ERR("%s: failed to fstat lock file: %s", name, strerror(errno));

1
tests/lockfile.at
View File

@@ -18,3 +18,4 @@ CHECK_LOCKFILE([lock_and_unlock_allows_other_process], [1])
CHECK_LOCKFILE([lock_timeout_gets_the_lock], [1])
CHECK_LOCKFILE([lock_timeout_runs_out], [1])
CHECK_LOCKFILE([lock_multiple], [0])
CHECK_LOCKFILE([lock_symlink], [0])

38
tests/test-lockfile.c
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
#include <errno.h>
#include <stdlib.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <unistd.h>
@@ -214,6 +215,40 @@ run_lock_multiple(void)
lockfile_unlock(a);
}
/* Checks that locking a dangling symlink works OK. (It used to hang.) */
static void
run_lock_symlink(void)
{
struct lockfile *a, *b, *dummy;
struct stat s;
/* Create a symlink .a.~lock~ pointing to .b.~lock~. */
CHECK(symlink(".b.~lock~", ".a.~lock~"), 0);
CHECK(lstat(".a.~lock~", &s), 0);
CHECK(S_ISLNK(s.st_mode) != 0, 1);
CHECK(stat(".a.~lock~", &s), -1);
CHECK(errno, ENOENT);
CHECK(stat(".b.~lock~", &s), -1);
CHECK(errno, ENOENT);
CHECK(lockfile_lock("a", 0, &a), 0);
CHECK(lockfile_lock("a", 0, &dummy), EDEADLK);
CHECK(lockfile_lock("b", 0, &dummy), EDEADLK);
lockfile_unlock(a);
CHECK(lockfile_lock("b", 0, &b), 0);
CHECK(lockfile_lock("b", 0, &dummy), EDEADLK);
CHECK(lockfile_lock("a", 0, &dummy), EDEADLK);
lockfile_unlock(b);
CHECK(lstat(".a.~lock~", &s), 0);
CHECK(S_ISLNK(s.st_mode) != 0, 1);
CHECK(stat(".a.~lock~", &s), 0);
CHECK(S_ISREG(s.st_mode) != 0, 1);
CHECK(stat(".b.~lock~", &s), 0);
CHECK(S_ISREG(s.st_mode) != 0, 1);
}
static void
run_help(void)
{
@@ -239,6 +274,7 @@ static const struct test tests[] = {
TEST(lock_timeout_gets_the_lock),
TEST(lock_timeout_runs_out),
TEST(lock_multiple),
TEST(lock_symlink),
TEST(help),
{ NULL, NULL }
#undef TEST