On Thu 19-12-24 12:53:01, Amir Goldstein wrote:
> Encoding file handles is usually performed by a filesystem >encode_fh()
> method that may fail for various reasons.
>
> The legacy users of exportfs_encode_fh(), namely, nfsd and
> name_to_handle_at(2) syscall are ready to cope with the possibility
> of failure to encode a file handle.
>
> There are a few other users of exportfs_encode_{fh,fid}() that
> currently have a WARN_ON() assertion when ->encode_fh() fails.
> Relax those assertions because they are wrong.
>
> The second linked bug report states commit 16aac5ad1fa9 ("ovl: support
> encoding non-decodable file handles") in v6.6 as the regressing commit,
> but this is not accurate.
>
> The aforementioned commit only increases the chances of the assertion
> and allows triggering the assertion with the reproducer using overlayfs,
> inotify and drop_caches.
>
> Triggering this assertion was always possible with other filesystems and
> other reasons of ->encode_fh() failures and more particularly, it was
> also possible with the exact same reproducer using overlayfs that is
> mounted with options index=on,nfs_export=on also on kernels < v6.6.
> Therefore, I am not listing the aforementioned commit as a Fixes commit.
>
> Backport hint: this patch will have a trivial conflict applying to
> v6.6.y, and other trivial conflicts applying to stable kernels < v6.6.
>
> Reported-by: syzbot+ec07f6f5ce62b858579f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Tested-by: syzbot+ec07f6f5ce62b858579f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/linux-unionfs/671fd40c.050a0220.4735a.024f.GAE@xxxxxxxxxx/
> Reported-by: Dmitry Safonov <dima@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-fsdevel/CAGrbwDTLt6drB9eaUagnQVgdPBmhLfqqxAf3F+Juqy_o6oP8uw@xxxxxxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Yeah, looks sensible. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
>
> Christian,
>
> I could have sumbitted two independant patches to relax the assertion
> in fsnotify and overlayfs via fsnotify and overlayfs trees, but the
> nature of the problem is the same and in both cases, the problem became
> worse with the introduction of non-decodable file handles support,
> so decided to fix them together and ask you to take the fix via the
> vfs tree.
>
> Please let you if you think it should be done differently.
>
> Thanks,
> Amir.
>
> fs/notify/fdinfo.c | 4 +---
> fs/overlayfs/copy_up.c | 5 ++---
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
> index dec553034027e..e933f9c65d904 100644
> --- a/fs/notify/fdinfo.c
> +++ b/fs/notify/fdinfo.c
> @@ -47,10 +47,8 @@ static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
> size = f->handle_bytes >> 2;
>
> ret = exportfs_encode_fid(inode, (struct fid *)f->f_handle, &size);
> - if ((ret == FILEID_INVALID) || (ret < 0)) {
> - WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
> + if ((ret == FILEID_INVALID) || (ret < 0))
> return;
> - }
>
> f->handle_type = ret;
> f->handle_bytes = size * sizeof(u32);
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 3601ddfeddc2e..56eee9f23ea9a 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -442,9 +442,8 @@ struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
> buflen = (dwords << 2);
>
> err = -EIO;
> - if (WARN_ON(fh_type < 0) ||
> - WARN_ON(buflen > MAX_HANDLE_SZ) ||
> - WARN_ON(fh_type == FILEID_INVALID))
> + if (fh_type < 0 || fh_type == FILEID_INVALID ||
> + WARN_ON(buflen > MAX_HANDLE_SZ))
> goto out_err;
>
> fh->fb.version = OVL_FH_VERSION;
> --
> 2.34.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
