locks: Fix dropped call to ->fl_release_private()

Prior to commit 4149be7bda7e, sys_flock() would allocate the file_lock
struct it was going to use to pass parameters, call ->flock() and then call
locks_free_lock() to get rid of it - which had the side effect of calling
locks_release_private() and thus ->fl_release_private().

With commit 4149be7bda7e, however, this is no longer the case: the struct
is now allocated on the stack, and locks_free_lock() is no longer called -
and thus any remaining private data doesn't get cleaned up either.

This causes afs flock to cause oops.  Kasan catches this as a UAF by the
list_del_init() in afs_fl_release_private() for the file_lock record
produced by afs_fl_copy_lock() as the original record didn't get delisted.
It can be reproduced using the generic/504 xfstest.

Fix this by reinstating the locks_release_private() call in sys_flock().
I'm not sure if this would affect any other filesystems.  If not, then the
release could be done in afs_flock() instead.

Changes
=======
ver #2)
 - Don't need to call ->fl_release_private() after calling the security
   hook, only after calling ->flock().

Fixes: 4149be7bda7e ("fs/lock: Don't allocate file_lock in flock_make_lock().")
cc: Chuck Lever <[email protected]>
cc: Jeff Layton <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
cc: [email protected]
Link: https://lore.kernel.org/r/166075758809.3532462.13307935588777587536.stgit@warthog.procyon.org.uk/
Acked-by: Kuniyuki Iwashima <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>

diff --git a/fs/locks.c b/fs/locks.c
index c266cfdc3291f92902481f8d9ed289ae08fe839c..607f94a0e789fc71407780e12580ca89891a1dad 100644 (file)
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2129,6 +2129,7 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
        else
                error = locks_lock_file_wait(f.file, &fl);
 
+       locks_release_private(&fl);
  out_putf:
        fdput(f);