On Sat, 2017-02-11 at 11:49 +0530, Pankaj Singh wrote:
> > During "fl_grant" callback wrong "block" will be compared hence
> > this will result in lock failure even if lock is actually granted.
>
> "nlm_compare_locks" will compare the locks on the bases of pid,
> owner,
> start offset, end offset and type. In case of posix threads as pid is
> same, also all other parameters can be same for locks of different
> files.
>
> Now the scenario is, if there are two posix thread with pid p1 and
> they try to take the lock on different files (say l1 and l2) then
> different blocks will be created (say b1 and b2). In this case
> underline filesystem may send "FILE_LOCK_DEFERRED" for both the locks
> and these blocks will be added to deferred block list.
>
> So during "fl_grant" callback lock are compared with the blocks of
> block list. Now lets say callback is called for l1 and the comparison
> will succeed with b1 (this is as expected) and B_GOT_CALLBACK flag
> will be set. But as b1 is still in block list, now when callback for
> l2 arrives then also comparison with b1 can succeed instead of b2
> because b1 is prior to b2 in the list. Hence B_GOT_CALLBACK flag will
> not be set for b2 and when NFS client will retry for lock then lock
> will be denied.
>
> Below is the snipt fl_grant code where comparison happens.
> list_for_each_entry(block, &nlm_blocked, b_list) {
> if (nlm_compare_locks(&block->b_call->a_args.lock.fl, fl)
OK. I see what you mean. You are saying, in essence, that in the lockd
server code, nlmsvc_notify_blocked() needs to check that the files are
the same (just like nlmsvc_lookup_block() already does).
I agree. That is a bug and it needs to be fixed. How about the
following?
Cheers
Trond
8<---------------------------------------------------------------
>From fff75e35ed857b9ad211905fee2acffa528696d9 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
Date: Sat, 11 Feb 2017 10:37:38 -0500
Subject: [PATCH] nlm: Ensure callback code also checks that the files match
It is not sufficient to just check that the lock pids match when
granting a callback, we also need to ensure that we're granting
the callback on the right file.
Reported-by: Pankaj Singh <psingh.ait@xxxxxxxxx>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
---
include/linux/lockd/lockd.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index c15373894a42..b37dee3acaba 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -355,7 +355,8 @@ static inline int nlm_privileged_requester(const struct svc_rqst *rqstp)
static inline int nlm_compare_locks(const struct file_lock *fl1,
const struct file_lock *fl2)
{
- return fl1->fl_pid == fl2->fl_pid
+ return file_inode(fl1->fl_file) == file_inode(fl2->fl_file)
+ && fl1->fl_pid == fl2->fl_pid
&& fl1->fl_owner == fl2->fl_owner
&& fl1->fl_start == fl2->fl_start
&& fl1->fl_end == fl2->fl_end
--
2.9.3
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
