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@xxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxx> wrote: > > > >>> > > > >>> From: Darrick J. Wong <djwong@xxxxxxxxxx> > > > >>> > > > >>> 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@xxxxxxxxxx> > > > >>> --- > > > >>> 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 > > > > > > > > > >
