Date: Sat, 3 Jul 2021 23:57:00 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: cc5020854d96 - stable/13 - fusefs: improve warnings about buggy FUSE servers Message-ID: <202107032357.163Nv0DI068009@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=cc5020854d9616f0c99f75a700eb41b473b1380f commit cc5020854d9616f0c99f75a700eb41b473b1380f Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2021-06-15 20:24:05 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2021-07-03 23:53:58 +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 a5f646e5f449..82e37ed5a466 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -870,9 +870,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, @@ -922,14 +919,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 3f23a35a8626..58adfde88871 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -576,16 +576,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 791ee9f38444..6b4a29214a96 100644 --- a/sys/fs/fuse/fuse_ipc.c +++ b/sys/fs/fuse/fuse_ipc.c @@ -1073,3 +1073,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 71562d9193ec..1351499c0712 100644 --- a/sys/fs/fuse/fuse_ipc.h +++ b/sys/fs/fuse/fuse_ipc.h @@ -233,6 +233,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) @@ -394,6 +399,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 7f47f8800994..b188996066d5 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/sys/fs/fuse/fuse_vfsops.c @@ -592,9 +592,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 a51c1b15e7f0..c79d8d5b5223 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -1177,8 +1177,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; @@ -1388,20 +1386,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); } @@ -2595,9 +2592,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?202107032357.163Nv0DI068009>