From 10d1a49c5ba29de81c8116a9d7505766a28f6fc0 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Thu, 3 Feb 2022 18:48:11 -0800 Subject: [PATCH] Fuse: `DeviceFD.Read`: Lock `DeviceFD` ahead of other locks. Syzkaller reports a "unlocking an unlocked mutex" here: https://github.com/google/gvisor/blob/a5ce865145c718f26c7a8f305f6c8262e992051c/pkg/sentry/fsimpl/fuse/dev.go#L154 ... which can only happen if `fd.fs` changes in the middle. By locking `DeviceFD` earlier, we follow the lock ordering specified in https://github.com/google/gvisor/blob/a5ce865145c718f26c7a8f305f6c8262e992051c/pkg/sentry/fsimpl/fuse/connection.go#L42 hopefully fixing the issue. Reported-by: syzbot+f211c42d52f93416457d@syzkaller.appspotmail.com PiperOrigin-RevId: 426288804 --- pkg/sentry/fsimpl/fuse/dev.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index c1bf1127a..a03170b85 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -149,6 +149,9 @@ func (fd *DeviceFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.R minBuffSize := linux.FUSE_MIN_READ_BUFFER inHdrLen := uint32((*linux.FUSEHeaderIn)(nil).SizeBytes()) writeHdrLen := uint32((*linux.FUSEWriteIn)(nil).SizeBytes()) + + fd.mu.Lock() + defer fd.mu.Unlock() fd.fs.conn.mu.Lock() negotiatedMinBuffSize := inHdrLen + writeHdrLen + fd.fs.conn.maxWrite fd.fs.conn.mu.Unlock() @@ -160,9 +163,6 @@ func (fd *DeviceFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.R if dst.NumBytes() < int64(minBuffSize) { return 0, linuxerr.EINVAL } - - fd.mu.Lock() - defer fd.mu.Unlock() return fd.readLocked(ctx, dst, opts) }