Re: [PATCH 7/7] fuse: enable FUSE_SYNCFS for all servers

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Aug 22, 2025 at 10:21:44AM -0700, Joanne Koong wrote:
> On Fri, Aug 22, 2025 at 4:32 AM Shachar Sharon <synarete@gmail.com> wrote:
> >
> > To the best of my understanding, there are two code paths which may
> > yield FUSE_SYNCFS: one from user-space syscall syncfs(2) and the other
> > from within the kernel itself. Unfortunately, there is no way to
> > distinguish between the two at sb->s_op->sync_fs level, and the DoS
> > argument refers to the second (kernel) case. If we could somehow
> > propagate this info all the way down to the fuse layer then I see no
> > reason for preventing (non-privileged) user-space programs from
> > calling syncfs(2) over FUSE mounted file-systems.
> 
> I interpreted the DoS comment as referring to the scenario where a
> userspace program calls generic sync()  and if an untrusted fuse
> server deliberately hangs on servicing that request then it'll hang
> sync forever. I think if this only affected the syncfs() syscall then
> it wouldn't be a problem since the caller is directly invoking it on a
> fuse fd, but if it affects generic sync() that seems like a big issue
> to me. Or at least that's my understanding of the code with
> ksys_sync() -> iterate_supers(sync_fs_one_sb, &wait).

<shrug> I think you can already DoS sync() (and by extension any other
place in the kernel where we try to flush out all filesystems in one go)
by dropping a FUSE_SETATTR call on the floor, because that's how we
flush dirty inodes to disk?  Or by doing the same for an FUSE_FSYNC
call?

--D

> Thanks,
> Joanne
> >
> >
> > Please correct me if I am wrong with my analysis.
> >
> >
> > - Shachar.
> >
> > On Fri, Aug 22, 2025 at 1:57 AM Bernd Schubert <bernd@bsbernd.com> wrote:
> > >
> > >
> > >
> > > On 8/22/25 00:28, Darrick J. Wong wrote:
> > > > On Thu, Aug 21, 2025 at 03:18:11PM -0700, Joanne Koong wrote:
> > > >> On Wed, Aug 20, 2025 at 5:52 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >>>
> > > >>> From: Darrick J. Wong <djwong@kernel.org>
> > > >>>
> > > >>> Turn on syncfs for all fuse servers so that the ones in the know can
> > > >>> flush cached intermediate data and logs to disk.
> > > >>>
> > > >>> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > > >>> ---
> > > >>>  fs/fuse/inode.c |    1 +
> > > >>>  1 file changed, 1 insertion(+)
> > > >>>
> > > >>>
> > > >>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > > >>> index 463879830ecf34..b05510799f93e1 100644
> > > >>> --- a/fs/fuse/inode.c
> > > >>> +++ b/fs/fuse/inode.c
> > > >>> @@ -1814,6 +1814,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> > > >>>                 if (!sb_set_blocksize(sb, ctx->blksize))
> > > >>>                         goto err;
> > > >>>  #endif
> > > >>> +               fc->sync_fs = 1;
> > > >>
> > > >> AFAICT, this enables syncfs only for fuseblk servers. Is this what you
> > > >> intended?
> > > >
> > > > I meant to say for all fuseblk servers, but TBH I can't see why you
> > > > wouldn't want to enable it for non-fuseblk servers too?
> > > >
> > > > (Maybe I was being overly cautious ;))
> > >
> > > Just checked, the initial commit message has
> > >
> > >
> > > <quote 2d82ab251ef0f6e7716279b04e9b5a01a86ca530>
> > > Note that such an operation allows the file server to DoS sync(). Since a
> > > typical FUSE file server is an untrusted piece of software running in
> > > userspace, this is disabled by default. Only enable it with virtiofs for
> > > now since virtiofsd is supposedly trusted by the guest kernel.
> > > </quote>
> > >
> > >
> > > With that we could at least enable for all privileged servers? And for
> > > non-privileged this could be an async?
> > >
> > >
> > > Thanks,
> > > Bernd
> > >
> > >
> > >
>