]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
really add fix for possible infinite loop w/ fcntl(F_SETLKW) over nfs
authorChris Wright <chrisw@sous-sol.org>
Thu, 17 Apr 2008 21:20:26 +0000 (14:20 -0700)
committerChris Wright <chrisw@sous-sol.org>
Thu, 17 Apr 2008 21:20:26 +0000 (14:20 -0700)
queue-2.6.24/locks-fix-possible-infinite-loop-in-fcntl-over-nfs.patch [new file with mode: 0644]

diff --git a/queue-2.6.24/locks-fix-possible-infinite-loop-in-fcntl-over-nfs.patch b/queue-2.6.24/locks-fix-possible-infinite-loop-in-fcntl-over-nfs.patch
new file mode 100644 (file)
index 0000000..e258c75
--- /dev/null
@@ -0,0 +1,117 @@
+From 19e729a928172103e101ffd0829fd13e68c13f78 Mon Sep 17 00:00:00 2001
+From: J. Bruce Fields <bfields@citi.umich.edu>
+Date: Mon, 14 Apr 2008 15:03:02 -0400
+Message-ID: <20080414190302.GM15950@fieldses.org>
+Subject: locks: fix possible infinite loop in fcntl(F_SETLKW) over nfs
+
+upstream commit: 19e729a928172103e101ffd0829fd13e68c13f78
+
+Miklos Szeredi found the bug:
+
+       "Basically what happens is that on the server nlm_fopen() calls
+       nfsd_open() which returns -EACCES, to which nlm_fopen() returns
+       NLM_LCK_DENIED.
+
+       "On the client this will turn into a -EAGAIN (nlm_stat_to_errno()),
+       which in will cause fcntl_setlk() to retry forever."
+
+So, for example, opening a file on an nfs filesystem, changing
+permissions to forbid further access, then trying to lock the file,
+could result in an infinite loop.
+
+And Trond Myklebust identified the culprit, from Marc Eshel and I:
+
+       7723ec9777d9832849b76475b1a21a2872a40d20 "locks: factor out
+       generic/filesystem switch from setlock code"
+
+That commit claimed to just be reshuffling code, but actually introduced
+a behavioral change by calling the lock method repeatedly as long as it
+returned -EAGAIN.
+
+We assumed this would be safe, since we assumed a lock of type SETLKW
+would only return with either success or an error other than -EAGAIN.
+However, nfs does can in fact return -EAGAIN in this situation, and
+independently of whether that behavior is correct or not, we don't
+actually need this change, and it seems far safer not to depend on such
+assumptions about the filesystem's ->lock method.
+
+Therefore, revert the problematic part of the original commit.  This
+leaves vfs_lock_file() and its other callers unchanged, while returning
+fcntl_setlk and fcntl_setlk64 to their former behavior.
+
+Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
+Tested-by: Miklos Szeredi <mszeredi@suse.cz>
+Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
+Cc: Marc Eshel <eshel@almaden.ibm.com>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Chris Wright <chrisw@sous-sol.org>
+---
+ fs/locks.c |   48 ++++++++++++++++++++++++++++--------------------
+ 1 file changed, 28 insertions(+), 20 deletions(-)
+
+--- a/fs/locks.c
++++ b/fs/locks.c
+@@ -1805,17 +1805,21 @@ again:
+       if (error)
+               goto out;
+-      for (;;) {
+-              error = vfs_lock_file(filp, cmd, file_lock, NULL);
+-              if (error != -EAGAIN || cmd == F_SETLK)
+-                      break;
+-              error = wait_event_interruptible(file_lock->fl_wait,
+-                              !file_lock->fl_next);
+-              if (!error)
+-                      continue;
++      if (filp->f_op && filp->f_op->lock != NULL)
++              error = filp->f_op->lock(filp, cmd, file_lock);
++      else {
++              for (;;) {
++                      error = posix_lock_file(filp, file_lock, NULL);
++                      if (error != -EAGAIN || cmd == F_SETLK)
++                              break;
++                      error = wait_event_interruptible(file_lock->fl_wait,
++                                      !file_lock->fl_next);
++                      if (!error)
++                              continue;
+-              locks_delete_block(file_lock);
+-              break;
++                      locks_delete_block(file_lock);
++                      break;
++              }
+       }
+       /*
+@@ -1929,17 +1933,21 @@ again:
+       if (error)
+               goto out;
+-      for (;;) {
+-              error = vfs_lock_file(filp, cmd, file_lock, NULL);
+-              if (error != -EAGAIN || cmd == F_SETLK64)
+-                      break;
+-              error = wait_event_interruptible(file_lock->fl_wait,
+-                              !file_lock->fl_next);
+-              if (!error)
+-                      continue;
++      if (filp->f_op && filp->f_op->lock != NULL)
++              error = filp->f_op->lock(filp, cmd, file_lock);
++      else {
++              for (;;) {
++                      error = posix_lock_file(filp, file_lock, NULL);
++                      if (error != -EAGAIN || cmd == F_SETLK64)
++                              break;
++                      error = wait_event_interruptible(file_lock->fl_wait,
++                                      !file_lock->fl_next);
++                      if (!error)
++                              continue;
+-              locks_delete_block(file_lock);
+-              break;
++                      locks_delete_block(file_lock);
++                      break;
++              }
+       }
+       /*