Date: Tue, 27 Jul 2021 16:49:58 GMT From: Alan Somers <asomers@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: ba99ff623b80 - stable/12 - fusefs: improve warnings about buggy FUSE servers Message-ID: <202107271649.16RGnwV1070246@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=ba99ff623b80d3b8173f6bd08758bc43626ea6d9 commit ba99ff623b80d3b8173f6bd08758bc43626ea6d9 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2021-06-15 20:24:05 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2021-07-27 16:20:46 +0000 fusefs: improve warnings about buggy FUSE servers The fusefs driver will print warning messages about FUSE servers that commit protocol violations. Previously it would print those warnings on every violation, but that could spam the console. Now it will print each warning no more than once per lifetime of the mount. There is also now a dtrace probe for each violation. Sponsored by: Axcient Reviewed by: emaste, pfg Differential Revision: https://reviews.freebsd.org/D30780 (cherry picked from commit 0b9a5c6fa1733e600dcdb3b9df3b3d0ad5996920) --- sys/fs/fuse/fuse_internal.c | 19 ++++++++----------- sys/fs/fuse/fuse_io.c | 8 ++++---- sys/fs/fuse/fuse_ipc.c | 14 ++++++++++++++ sys/fs/fuse/fuse_ipc.h | 8 ++++++++ sys/fs/fuse/fuse_vfsops.c | 23 ++++++++++++++++++++--- sys/fs/fuse/fuse_vnops.c | 18 +++++++++--------- 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 57ff01cd70b7..f70533b54028 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -871,9 +871,6 @@ fuse_internal_forget_send(struct mount *mp, fdisp_destroy(&fdi); } -SDT_PROBE_DEFINE2(fusefs, , internal, getattr_cache_incoherent, - "struct vnode*", "struct fuse_attr_out*"); - /* Fetch the vnode's attributes from the daemon*/ int fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap, @@ -923,14 +920,14 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap, * The server changed the file's size even though we had it * cached! That's a server bug. */ - SDT_PROBE2(fusefs, , internal, getattr_cache_incoherent, vp, - fao); - printf("%s: cache incoherent on %s! " - "Buggy FUSE server detected. To prevent data corruption, " - "disable the data cache by mounting with -o direct_io, or " - "as directed otherwise by your FUSE server's " - "documentation\n", __func__, - vnode_mount(vp)->mnt_stat.f_mntonname); + struct mount *mp = vnode_mount(vp); + struct fuse_data *data = fuse_get_mpdata(mp); + + fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT, + "cache incoherent! " + "To prevent data corruption, disable the data cache " + "by mounting with -o direct_io, or as directed " + "otherwise by your FUSE server's documentation."); int iosize = fuse_iosize(vp); v_inval_buf_range(vp, 0, INT64_MAX, iosize); } diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c index ea599e9aab34..5769ac8eccd1 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -616,16 +616,16 @@ retry: fvdat->flag &= ~FN_SIZECHANGE; if (diff < 0) { - printf("WARNING: misbehaving FUSE filesystem " - "wrote more data than we provided it\n"); + fuse_warn(data, FSESS_WARN_WROTE_LONG, + "wrote more data than we provided it."); err = EINVAL; break; } else if (diff > 0) { /* Short write */ if (!direct_io) { - printf("WARNING: misbehaving FUSE filesystem: " + fuse_warn(data, FSESS_WARN_SHORT_WRITE, "short writes are only allowed with " - "direct_io\n"); + "direct_io."); } if (ioflag & IO_DIRECT) { /* Return early */ diff --git a/sys/fs/fuse/fuse_ipc.c b/sys/fs/fuse/fuse_ipc.c index 514f7857faf5..e3be63b9d6d4 100644 --- a/sys/fs/fuse/fuse_ipc.c +++ b/sys/fs/fuse/fuse_ipc.c @@ -1095,3 +1095,17 @@ fuse_ipc_destroy(void) counter_u64_free(fuse_ticket_count); uma_zdestroy(ticket_zone); } + +SDT_PROBE_DEFINE3(fusefs,, ipc, warn, "struct fuse_data*", "unsigned", "char*"); +void +fuse_warn(struct fuse_data *data, unsigned flag, const char *msg) +{ + SDT_PROBE3(fusefs, , ipc, warn, data, flag, msg); + if (!(data->dataflags & flag)) { + printf("WARNING: FUSE protocol violation for server mounted at " + "%s: %s " + "This warning will not be repeated.\n", + data->mp->mnt_stat.f_mntonname, msg); + data->dataflags |= flag; + } +} diff --git a/sys/fs/fuse/fuse_ipc.h b/sys/fs/fuse/fuse_ipc.h index bedecefd157a..8e041529e119 100644 --- a/sys/fs/fuse/fuse_ipc.h +++ b/sys/fs/fuse/fuse_ipc.h @@ -231,6 +231,11 @@ struct fuse_data { #define FSESS_POSIX_LOCKS 0x2000 /* daemon supports POSIX locks */ #define FSESS_EXPORT_SUPPORT 0x10000 /* daemon supports NFS-style lookups */ #define FSESS_INTR 0x20000 /* interruptible mounts */ +#define FSESS_WARN_SHORT_WRITE 0x40000 /* Short write without direct_io */ +#define FSESS_WARN_WROTE_LONG 0x80000 /* Wrote more data than provided */ +#define FSESS_WARN_LSEXTATTR_LONG 0x100000 /* Returned too many extattrs */ +#define FSESS_WARN_CACHE_INCOHERENT 0x200000 /* Read cache incoherent */ +#define FSESS_WARN_WB_CACHE_INCOHERENT 0x400000 /* WB cache incoherent */ #define FSESS_MNTOPTS_MASK ( \ FSESS_DAEMON_CAN_SPY | FSESS_PUSH_SYMLINKS_IN | \ FSESS_DEFAULT_PERMISSIONS | FSESS_INTR) @@ -365,6 +370,9 @@ fuse_libabi_geq(struct fuse_data *data, uint32_t abi_maj, uint32_t abi_min) data->fuse_libabi_minor >= abi_min)); } +/* Print msg as a warning to the console, but no more than once per session */ +void fuse_warn(struct fuse_data *data, unsigned flag, const char *msg); + struct fuse_data *fdata_alloc(struct cdev *dev, struct ucred *cred); void fdata_trydestroy(struct fuse_data *data); void fdata_set_dead(struct fuse_data *data); diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c index a31b95df3318..a4a0ce3b86d0 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/sys/fs/fuse/fuse_vfsops.c @@ -590,9 +590,26 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp) if (vnode_isreg(*vpp) && filesize != fvdat->cached_attrs.va_size && fvdat->flag & FN_SIZECHANGE) { - printf("%s: WB cache incoherent on %s!\n", __func__, - vnode_mount(*vpp)->mnt_stat.f_mntonname); - + if (data->cache_mode == fuse_data_cache_mode) { + const char *msg; + + if (fuse_libabi_geq(data, 7, 23)) { + msg = "writeback cache incoherent!." + "To prevent data corruption, disable " + "the writeback cache according to your " + "FUSE server's documentation."; + } else { + msg = "writeback cache incoherent!." + "To prevent data corruption, disable " + "the writeback cache by setting " + "vfs.fusefs.data_cache_mode to 0 or 1."; + } + fuse_warn(data, FSESS_WARN_WB_CACHE_INCOHERENT, msg); + } else { + /* If we get here, it's likely a fusefs kernel bug */ + printf("%s: WB cache incoherent on %s!\n", __func__, + vnode_mount(*vpp)->mnt_stat.f_mntonname); + } fvdat->flag &= ~FN_SIZECHANGE; } diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index d30fc32d1dc5..1c1ac7680f0e 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -967,8 +967,6 @@ fuse_lookup_alloc(struct mount *mp, void *arg, int lkflags, struct vnode **vpp) SDT_PROBE_DEFINE3(fusefs, , vnops, cache_lookup, "int", "struct timespec*", "struct timespec*"); -SDT_PROBE_DEFINE2(fusefs, , vnops, lookup_cache_incoherent, - "struct vnode*", "struct fuse_entry_out*"); /* struct vnop_lookup_args { struct vnodeop_desc *a_desc; @@ -1177,20 +1175,19 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) ((vap = VTOVA(vp)) && filesize != vap->va_size))) { - SDT_PROBE2(fusefs, , vnops, lookup_cache_incoherent, vp, feo); fvdat->flag &= ~FN_SIZECHANGE; /* * The server changed the file's size even * though we had it cached, or had dirty writes * in the WB cache! */ - printf("%s: cache incoherent on %s! " - "Buggy FUSE server detected. To prevent " + fuse_warn(data, FSESS_WARN_CACHE_INCOHERENT, + "cache incoherent! " + "To prevent " "data corruption, disable the data cache " "by mounting with -o direct_io, or as " "directed otherwise by your FUSE server's " - "documentation\n", __func__, - vnode_mount(vp)->mnt_stat.f_mntonname); + "documentation."); int iosize = fuse_iosize(vp); v_inval_buf_range(vp, 0, INT64_MAX, iosize); } @@ -2357,9 +2354,12 @@ fuse_vnop_listextattr(struct vop_listextattr_args *ap) linux_list = fdi.answ; /* FUSE doesn't allow the server to return more data than requested */ if (fdi.iosize > linux_list_len) { - printf("WARNING: FUSE protocol violation. Server returned " + struct fuse_data *data = fuse_get_mpdata(mp); + + fuse_warn(data, FSESS_WARN_LSEXTATTR_LONG, + "server returned " "more extended attribute data than requested; " - "should've returned ERANGE instead"); + "should've returned ERANGE instead."); } else { /* But returning less data is fine */ linux_list_len = fdi.iosize;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107271649.16RGnwV1070246>