linux-kernel - [PATCH] filelock: fix deadlock detection in POSIX locking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Sun, 18 Feb 2024 08:33:28 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Christian Brauner <brauner@...nel.org>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, NeilBrown <neilb@...e.de>, 
 Alexander Aring <alex.aring@...il.com>, 
 Chuck Lever <chuck.lever@...cle.com>, Jan Kara <jack@...e.cz>, 
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
 kernel test robot <oliver.sang@...el.com>, Jeff Layton <jlayton@...nel.org>
Subject: [PATCH] filelock: fix deadlock detection in POSIX locking

The FL_POSIX check in __locks_insert_block was inadvertantly broken
recently and is now inserting only OFD locks instead of only legacy
POSIX locks.

This breaks deadlock detection in POSIX locks, and may also be the root
cause of a performance regression noted by the kernel test robot.
Restore the proper sense of the test.

Fixes: b6be3714005c ("filelock: convert __locks_insert_block, conflict and deadlock checks to use file_lock_core")
Reported-by: kernel test robot <oliver.sang@...el.com>
Closes: https://lore.kernel.org/oe-lkp/202402181229.f8147f40-oliver.sang@intel.com
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
Disregard what I said earlier about this bug being harmless. It broke
deadlock detection in POSIX locks (LTP fcntl17 shows the bug). This
patch fixes it. It may be best to squash this into the patch that
introduced the regression.

I'm not certain if this fixes the performance regression that the KTR
noticed recently in this patch, but that's what got me looking more
closely, so I'll give it credit for reporting this. Hopefully it'll
confirm that result for us.
---
 fs/locks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index 26d52ef5314a..90c8746874de 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -812,7 +812,7 @@ static void __locks_insert_block(struct file_lock_core *blocker,
 	list_add_tail(&waiter->flc_blocked_member,
 		      &blocker->flc_blocked_requests);
 
-	if ((blocker->flc_flags & (FL_POSIX|FL_OFDLCK)) == (FL_POSIX|FL_OFDLCK))
+	if ((blocker->flc_flags & (FL_POSIX|FL_OFDLCK)) == FL_POSIX)
 		locks_insert_global_blocked(waiter);
 
 	/* The requests in waiter->flc_blocked are known to conflict with

---
base-commit: 292fcaa1f937345cb65f3af82a1ee6692c8df9eb
change-id: 20240218-flsplit4-e843536f4c11

Best regards,
-- 
Jeff Layton <jlayton@...nel.org>