linux-kernel - Re: [PATCH v2] fs: drop assert in file_seek_cur_needs_f

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-prev] [day] [month] [year] [list]

Message-ID: <CAGudoHEu2v4MisyGO6gcBcnfKMgK41Y1=syhZoPm4exvj4fLQA@mail.gmail.com>
Date: Fri, 13 Jun 2025 12:35:24 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Luis Henriques <luis@...lia.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kernel-dev@...lia.com
Subject: Re: [PATCH v2] fs: drop assert in file_seek_cur_needs_f_lock

On Fri, Jun 13, 2025 at 12:11 PM Luis Henriques <luis@...lia.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@...e.cz>
> Suggested-by: Mateusz Guzik <mjguzik@...il.com>
> Signed-off-by: Luis Henriques <luis@...lia.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>