From owner-dev-commits-src-main@freebsd.org Wed Jun 16 19:32:13 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 0EF236415E3; Wed, 16 Jun 2021 19:32:13 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4G4wLN72cqz3Gsh; Wed, 16 Jun 2021 19:32:12 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D9CB521728; Wed, 16 Jun 2021 19:32:12 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 15GJWC1W062236; Wed, 16 Jun 2021 19:32:12 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 15GJWCUs062235; Wed, 16 Jun 2021 19:32:12 GMT (envelope-from git) Date: Wed, 16 Jun 2021 19:32:12 GMT Message-Id: <202106161932.15GJWCUs062235@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Alan Somers Subject: git: 0b9a5c6fa173 - main - fusefs: improve warnings about buggy FUSE servers MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: asomers X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 0b9a5c6fa1733e600dcdb3b9df3b3d0ad5996920 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jun 2021 19:32:13 -0000 The branch main has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=0b9a5c6fa1733e600dcdb3b9df3b3d0ad5996920 commit 0b9a5c6fa1733e600dcdb3b9df3b3d0ad5996920 Author: Alan Somers AuthorDate: 2021-06-15 20:24:05 +0000 Commit: Alan Somers CommitDate: 2021-06-16 19:31:31 +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. MFC after: 2 weeks Sponsored by: Axcient Reviewed by: emaste, pfg Differential Revision: https://reviews.freebsd.org/D30780 --- 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 bcf6e5f601bf..47c7fb97e556 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;