LVM2 ./WHATS_NEW lib/locking/file_locking.c

LVM2 ./WHATS_NEW lib/locking/file_locking.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-02-08 11:17:35

Modified files:
	.              : WHATS_NEW 
	lib/locking    : file_locking.c 

Log message:
	Check that whole locking_dir fits _lock_dir buffer

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW.diff?cvsroot=lvm2&r1=1.2264&r2=1.2265
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/lib/locking/file_locking.c.diff?cvsroot=lvm2&r1=1.62&r2=1.63

--- LVM2/WHATS_NEW	2012/02/08 11:12:18	1.2264
+++ LVM2/WHATS_NEW	2012/02/08 11:17:34	1.2265
@@ -1,5 +1,6 @@
 Version 2.02.91 -
 ===================================
+  Check that whole locking_dir fits _lock_dir buffer in init_file_locking().
   Use list functions for label_exit().
   Ensure strncpy() function always ends with '\0'.
   Set status in _fsadm_cmd() for error path.
--- LVM2/lib/locking/file_locking.c	2012/01/20 00:27:20	1.62
+++ LVM2/lib/locking/file_locking.c	2012/02/08 11:17:35	1.63
@@ -337,6 +337,7 @@
 		      int suppress_messages)
 {
 	int r;
+	const char *locking_dir;
 
 	locking->lock_resource = _file_lock_resource;
 	locking->reset_locking = _reset_file_locking;
@@ -344,9 +345,14 @@
 	locking->flags = 0;
 
 	/* Get lockfile directory from config file */
-	strncpy(_lock_dir, find_config_tree_str(cmd, "global/locking_dir",
-						DEFAULT_LOCK_DIR),
-		sizeof(_lock_dir));
+	locking_dir = find_config_tree_str(cmd, "global/locking_dir",
+					   DEFAULT_LOCK_DIR);
+	if (strlen(locking_dir) >= sizeof(_lock_dir)) {
+		log_error("Path for locking_dir %s is invalid.", locking_dir);
+		return 0;
+	}
+
+	strcpy(_lock_dir, locking_dir);
 
 	_prioritise_write_locks =
 	    find_config_tree_bool(cmd, "global/prioritise_write_locks",

LVM2 ./WHATS_NEW lib/locking/file_locking.c

[mornfall]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2009-08-13 13:23:52

Modified files:
	.              : WHATS_NEW 
	lib/locking    : file_locking.c 

Log message:
	Refactor file locking, lifting the flock wrapper code into separate
	functions. Also fixes a bug, where a nonblocking lock could, in certain race
	situations, succeed without actually obtaining the lock.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW.diff?cvsroot=lvm2&r1=1.1237&r2=1.1238
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/lib/locking/file_locking.c.diff?cvsroot=lvm2&r1=1.41&r2=1.42

--- LVM2/WHATS_NEW	2009/08/13 12:19:30	1.1237
+++ LVM2/WHATS_NEW	2009/08/13 13:23:51	1.1238
@@ -1,5 +1,6 @@
 Version 2.02.52 -
 =================================
+  Fix bug where non-blocking file locks could be granted in error.
   Make lvm2app pv_t, lv_t, vg_t handle definitions consistent with lvm_t.
   Fix vgextend error path - if ORPHAN lock fails, unlock / release vg (2.02.49).
   Fix compile warning in clvmd.
--- LVM2/lib/locking/file_locking.c	2008/11/12 09:30:52	1.41
+++ LVM2/lib/locking/file_locking.c	2009/08/13 13:23:52	1.42
@@ -43,13 +43,26 @@
 static sigset_t _fullsigset, _intsigset;
 static volatile sig_atomic_t _handler_installed;
 
+static void _undo_flock(const char *file, int fd)
+{
+	struct stat buf1, buf2;
+
+	if (!flock(fd, LOCK_NB | LOCK_EX) &&
+	    !stat(file, &buf1) &&
+	    !fstat(fd, &buf2) &&
+	    is_same_inode(buf1, buf2))
+		if (unlink(file))
+			log_sys_error("unlink", file);
+
+	if (close(fd) < 0)
+		log_sys_error("close", file);
+}
+
 static int _release_lock(const char *file, int unlock)
 {
 	struct lock_list *ll;
 	struct dm_list *llh, *llt;
 
-	struct stat buf1, buf2;
-
 	dm_list_iterate_safe(llh, llt, &_lock_list) {
 		ll = dm_list_item(llh, struct lock_list);
 
@@ -61,15 +74,7 @@
 					log_sys_error("flock", ll->res);
 			}
 
-			if (!flock(ll->lf, LOCK_NB | LOCK_EX) &&
-			    !stat(ll->res, &buf1) &&
-			    !fstat(ll->lf, &buf2) &&
-			    is_same_inode(buf1, buf2))
-				if (unlink(ll->res))
-					log_sys_error("unlink", ll->res);
-
-			if (close(ll->lf) < 0)
-				log_sys_error("close", ll->res);
+			_undo_flock(ll->res, ll->lf);
 
 			dm_free(ll->res);
 			dm_free(llh);
@@ -124,14 +129,53 @@
 	siginterrupt(SIGINT, 1);
 }
 
-static int _lock_file(const char *file, uint32_t flags)
+static int _do_flock(const char *file, int *fd, int operation, uint32_t nonblock)
 {
-	int operation;
 	int r = 1;
 	int old_errno;
+	struct stat buf1, buf2;
+
+	do {
+		if ((*fd > -1) && close(*fd))
+			log_sys_error("close", file);
+
+		if ((*fd = open(file, O_CREAT | O_APPEND | O_RDWR, 0777)) < 0) {
+			log_sys_error("open", file);
+			return 0;
+		}
+
+		if (nonblock)
+			operation |= LOCK_NB;
+		else
+			_install_ctrl_c_handler();
+
+		r = flock(*fd, operation);
+		old_errno = errno;
+		if (!nonblock)
+			_remove_ctrl_c_handler();
+
+		if (r) {
+			errno = old_errno;
+			log_sys_error("flock", file);
+			close(*fd);
+			return 0;
+		}
+
+		if (!stat(file, &buf1) && !fstat(*fd, &buf2) &&
+		    is_same_inode(buf1, buf2))
+			return 1;
+	} while (!nonblock);
+
+	return_0;
+}
+
+static int _lock_file(const char *file, uint32_t flags)
+{
+	int operation;
+	uint32_t nonblock = flags & LCK_NONBLOCK;
+	int r;
 
 	struct lock_list *ll;
-	struct stat buf1, buf2;
 	char state;
 
 	switch (flags & LCK_TYPE_MASK) {
@@ -151,56 +195,28 @@
 	}
 
 	if (!(ll = dm_malloc(sizeof(struct lock_list))))
-		return 0;
+		return_0;
 
 	if (!(ll->res = dm_strdup(file))) {
 		dm_free(ll);
-		return 0;
+		return_0;
 	}
 
 	ll->lf = -1;
 
 	log_very_verbose("Locking %s %c%c", ll->res, state,
-			 flags & LCK_NONBLOCK ? ' ' : 'B');
-	do {
-		if ((ll->lf > -1) && close(ll->lf))
-			log_sys_error("close", file);
-
-		if ((ll->lf = open(file, O_CREAT | O_APPEND | O_RDWR, 0777))
-		    < 0) {
-			log_sys_error("open", file);
-			goto err;
-		}
+			 nonblock ? ' ' : 'B');
 
-		if ((flags & LCK_NONBLOCK))
-			operation |= LOCK_NB;
-		else
-			_install_ctrl_c_handler();
-
-		r = flock(ll->lf, operation);
-		old_errno = errno;
-		if (!(flags & LCK_NONBLOCK))
-			_remove_ctrl_c_handler();
-
-		if (r) {
-			errno = old_errno;
-			log_sys_error("flock", ll->res);
-			close(ll->lf);
-			goto err;
-		}
-
-		if (!stat(ll->res, &buf1) && !fstat(ll->lf, &buf2) &&
-		    is_same_inode(buf1, buf2))
-			break;
-	} while (!(flags & LCK_NONBLOCK));
-
-	dm_list_add(&_lock_list, &ll->list);
-	return 1;
+	r = _do_flock(file, &ll->lf, operation, nonblock);
+	if (r)
+		dm_list_add(&_lock_list, &ll->list);
+	else {
+		dm_free(ll->res);
+		dm_free(ll);
+		stack;
+	}
 
-      err:
-	dm_free(ll->res);
-	dm_free(ll);
-	return 0;
+	return r;
 }
 
 static int _file_lock_resource(struct cmd_context *cmd, const char *resource,

LVM2 ./WHATS_NEW lib/locking/file_locking.c

[meyering]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	meyering@sourceware.org	2007-07-20 12:12:52

Modified files:
	.              : WHATS_NEW 
	lib/locking    : file_locking.c 

Log message:
	Don't leak a file descriptor in _lock_file when flock fails.
	
	* lib/locking/file_locking.c (_lock_file): Close fd upon flock failure.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW.diff?cvsroot=lvm2&r1=1.661&r2=1.662
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/lib/locking/file_locking.c.diff?cvsroot=lvm2&r1=1.26&r2=1.27

--- LVM2/WHATS_NEW	2007/07/19 07:06:47	1.661
+++ LVM2/WHATS_NEW	2007/07/20 12:12:52	1.662
@@ -1,5 +1,6 @@
 Version 2.02.28 -
 ================================
+  Don't leak a file descriptor in _lock_file(), when flock fails.
   Add SUN's LDOM virtual block device to filters
   Split metadata-external.h out from metadata.h for the tools to use.
 
@@ -928,7 +929,7 @@
 
 You need to update libdevmapper before using 'vgmknodes' or 'vgscan --mknodes'.
 If your root filesystem is on an LV, you should run one of those two
-commands to fix up the special files in /dev in your real root filesystem 
+commands to fix up the special files in /dev in your real root filesystem
 after finishing with your initrd.  Also, remember you can use
 'vgchange --ignorelockingfailure' on your initrd if the tool fails because
 it can't write a lock file to a read-only filesystem.
@@ -938,7 +939,7 @@
 A pvmove implementation is now available for the new metadata format.
 
 When running a command that allocates space (e.g. lvcreate), you can now
-restrict not only which disk(s) may be used but also the Physical Extents 
+restrict not only which disk(s) may be used but also the Physical Extents
 on those disks.  e.g. lvcreate -L 10 vg1 /dev/hda6:1000-2000:3000-4000
 
 
@@ -950,12 +951,12 @@
   It's more compact and supports transactional changes and replication.
   Should things go wrong on a system, it's human-readable (and editable).
 
-Please report any problems you find to the mailing list, 
+Please report any problems you find to the mailing list,
 linux-lvm@sistina.com.  The software has NOT yet been thoroughly
 tested and so quite possibly there'll still be some bugs in it.
 Be aware of the disclaimer in the COPYING file.
 
-While testing, we recommend turning logging on in the configuration file 
+While testing, we recommend turning logging on in the configuration file
 to provide us with diagnostic information:
   log {
         file="/tmp/lvm2.log"
@@ -966,7 +967,7 @@
 You should schedule regular backups of your configuration file and
 metadata backups and archives (normally kept under /etc/lvm).
 
-Please read docs/example.conf and "man lvm.conf" to find out more about 
+Please read docs/example.conf and "man lvm.conf" to find out more about
 the configuration file.
 
 To convert an existing volume group called vg1 to the new format using
@@ -995,7 +996,7 @@
 LVM2 maintains a backup of the current metadata for each volume group
 in /etc/lvm/backup, and puts copies of previous versions in
 /etc/lvm/archive.  "vgcfgbackup" and "vgcfgrestore" can be used to
-create and restore from these files.  If you fully understand what 
+create and restore from these files.  If you fully understand what
 you're doing, metadata can be changed by editing a copy of a current
 backup file and using vgcfgrestore to reload it.
 
@@ -1012,8 +1013,8 @@
 The internal cache.  If you turn on debugging output you'll see lots of
 repeated messages, many of which will eventually get optimised out.
 
---test sometimes causes a command to fail (e.g. vgconvert --test) even 
-though the real command would work: again, fixing this is waiting for 
+--test sometimes causes a command to fail (e.g. vgconvert --test) even
+though the real command would work: again, fixing this is waiting for
 the work on the cache.
 
 Several of the tools do not yet contain the logic to handle full
@@ -1026,4 +1027,3 @@
 
 Recovery tools to salvage "lost" metadata directly from the disks:
 but we hope the new format will mean such tools are hardly ever needed!
-
--- LVM2/lib/locking/file_locking.c	2007/01/25 14:37:48	1.26
+++ LVM2/lib/locking/file_locking.c	2007/07/20 12:12:52	1.27
@@ -185,6 +185,7 @@
 		if (r) {
 			errno = old_errno;
 			log_sys_error("flock", ll->res);
+			close(ll->lf);
 			goto err;
 		}