The assert in function file_seek_cur_needs_f_lock() can be triggered very
easily because there are many users of vfs_llseek() (such as overlayfs)
that do their custom locking around llseek instead of relying on
fdget_pos(). Just drop the overzealous assertion.

Fixes: da06e3c51794 ("fs: don't needlessly acquire f_lock")
Suggested-by: Jan Kara <jack@suse.cz>
Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: Luis Henriques <luis@igalia.com>
---
Hi!

As suggested by Mateusz, I'm adding a comment (also suggested by him!) to
replace the assertion.  I'm also adding the 'Suggested-by:' tags, although
I'm not sure it's the correct tag to use -- the authorship of this patch
isn't really clear at this point :-)

 fs/file.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index 3a3146664cf3..b6db031545e6 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -1198,8 +1198,12 @@ bool file_seek_cur_needs_f_lock(struct file *file)
 	if (!(file->f_mode & FMODE_ATOMIC_POS) && !file->f_op->iterate_shared)
 		return false;
 
-	VFS_WARN_ON_ONCE((file_count(file) > 1) &&
-			 !mutex_is_locked(&file->f_pos_lock));
+	/*
+	 * Note that we are not guaranteed to be called after fdget_pos() on
+	 * this file obj, in which case the caller is expected to provide the
+	 * appropriate locking.
+	 */
+
 	return true;
 }

On Fri, 13 Jun 2025 11:11:11 +0100, Luis Henriques wrote:
> The assert in function file_seek_cur_needs_f_lock() can be triggered very
> easily because there are many users of vfs_llseek() (such as overlayfs)
> that do their custom locking around llseek instead of relying on
> fdget_pos(). Just drop the overzealous assertion.
> 
> 

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] fs: drop assert in file_seek_cur_needs_f_lock
      https://git.kernel.org/vfs/vfs/c/dd2d6b7f6f51

On Fri, Jun 13, 2025 at 12:11 PM Luis Henriques <luis@igalia.com> wrote:
>
> The assert in function file_seek_cur_needs_f_lock() can be triggered very
> easily because there are many users of vfs_llseek() (such as overlayfs)
> that do their custom locking around llseek instead of relying on
> fdget_pos(). Just drop the overzealous assertion.
>
> Fixes: da06e3c51794 ("fs: don't needlessly acquire f_lock")
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Mateusz Guzik <mjguzik@gmail.com>
> Signed-off-by: Luis Henriques <luis@igalia.com>
> ---
> Hi!
>
> As suggested by Mateusz, I'm adding a comment (also suggested by him!) to
> replace the assertion.  I'm also adding the 'Suggested-by:' tags, although
> I'm not sure it's the correct tag to use -- the authorship of this patch
> isn't really clear at this point :-)
>
>  fs/file.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index 3a3146664cf3..b6db031545e6 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -1198,8 +1198,12 @@ bool file_seek_cur_needs_f_lock(struct file *file)
>         if (!(file->f_mode & FMODE_ATOMIC_POS) && !file->f_op->iterate_shared)
>                 return false;
>
> -       VFS_WARN_ON_ONCE((file_count(file) > 1) &&
> -                        !mutex_is_locked(&file->f_pos_lock));
> +       /*
> +        * Note that we are not guaranteed to be called after fdget_pos() on
> +        * this file obj, in which case the caller is expected to provide the
> +        * appropriate locking.
> +        */
> +
>         return true;
>  }
>

well i think this is fine, obviously ;-)

I was hoping a native speaker would do some touch ups on the comment though.
-- 
Mateusz Guzik <mjguzik gmail.com>