[PATCH 2/4] locks: don't unnecessarily fail posix lock operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



posix_lock_file() was too cautious, failing operations on OOM, even if
they didn't actually require an allocation.

This has the disadvantage, that a failing unlock on process exit could
lead to a memory leak.  There are two possibilites for this:

- filesystem implements .lock() and calls back to posix_lock_file().
On cleanup of files_struct locks_remove_posix() is called which should
remove all locks belonging to files_struct.  However if filesystem
calls posix_lock_file() which fails, then those locks will never be
freed.

- if a file is closed while a lock is blocked, then after acquiring
fcntl_setlk() will undo the lock.  But this unlock itself might fail
on OOM, again possibly leaking the lock.

The solution is to move the checking of the allocations until after it
is sure that they will be needed.  This will solve the above problem
since unlock will always succeed unless it splits an existing region.

Signed-off-by: Miklos Szeredi <miklos@xxxxxxxxxx>

Index: linux/fs/locks.c
===================================================================
--- linux.orig/fs/locks.c	2006-03-31 18:55:33.000000000 +0200
+++ linux/fs/locks.c	2006-03-31 18:55:33.000000000 +0200
@@ -835,14 +835,7 @@ int __posix_lock_file(struct inode *inod
 	if (request->fl_flags & FL_ACCESS)
 		goto out;
 
-	error = -ENOLCK; /* "no luck" */
-	if (!(new_fl && new_fl2))
-		goto out;
-
 	/*
-	 * We've allocated the new locks in advance, so there are no
-	 * errors possible (and no blocking operations) from here on.
-	 * 
 	 * Find the first old lock with the same owner as the new lock.
 	 */
 	
@@ -939,6 +932,18 @@ int __posix_lock_file(struct inode *inod
 		before = &fl->fl_next;
 	}
 
+	/*
+	 * The above code only modifies existing locks in case of
+	 * merging or replacing.  If new lock(s) need to be inserted
+	 * all modifications are done bellow this, so it's safe yet to
+	 * bail out.
+	 */
+	error = -ENOLCK; /* "no luck" */
+	if (!added && request->fl_type != F_UNLCK && !new_fl)
+		goto out;
+	if (right && left == right && !new_fl2)
+		goto out;
+
 	error = 0;
 	if (!added) {
 		if (request->fl_type == F_UNLCK)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux