From fa8f71f2ec5a5c701ac498b027915d39d8a0147f Mon Sep 17 00:00:00 2001
From: Jamie Liu <jamieliu@google.com>
Date: Mon, 4 Apr 2022 13:30:52 -0700
Subject: [PATCH] Check mount writability in VFS2 [f]access[at][2]
 implementations.

Linux exempts special files (sockets, named pipes, and character/block device
special files) from mount writability checks in both open
(fs/open.c:do_dentry_open()) and access (fs/open.c:do_faccessat()) paths. We
don't currently do so in open (in VFS1 or VFS2) or access (in VFS1), so we
don't do so in VFS2 access either.

PiperOrigin-RevId: 439398002
---
 pkg/sentry/fsimpl/gofer/filesystem.go   | 8 +++++++-
 pkg/sentry/fsimpl/kernfs/filesystem.go  | 8 +++++++-
 pkg/sentry/fsimpl/overlay/filesystem.go | 8 +++++++-
 pkg/sentry/fsimpl/tmpfs/filesystem.go   | 8 +++++++-
 test/syscalls/linux/mount.cc            | 2 ++
 5 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index bcce69a19..984cdf008 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -763,7 +763,13 @@ func (fs *filesystem) AccessAt(ctx context.Context, rp *vfs.ResolvingPath, creds
 	if err != nil {
 		return err
 	}
-	return d.checkPermissions(creds, ats)
+	if err := d.checkPermissions(creds, ats); err != nil {
+		return err
+	}
+	if ats.MayWrite() && rp.Mount().ReadOnly() {
+		return linuxerr.EROFS
+	}
+	return nil
 }
 
 // GetDentryAt implements vfs.FilesystemImpl.GetDentryAt.
diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go
index 9a132930a..9afa93333 100644
--- a/pkg/sentry/fsimpl/kernfs/filesystem.go
+++ b/pkg/sentry/fsimpl/kernfs/filesystem.go
@@ -311,7 +311,13 @@ func (fs *Filesystem) AccessAt(ctx context.Context, rp *vfs.ResolvingPath, creds
 	if err != nil {
 		return err
 	}
-	return d.inode.CheckPermissions(ctx, creds, ats)
+	if err := d.inode.CheckPermissions(ctx, creds, ats); err != nil {
+		return err
+	}
+	if ats.MayWrite() && rp.Mount().ReadOnly() {
+		return linuxerr.EROFS
+	}
+	return nil
 }
 
 // GetDentryAt implements vfs.FilesystemImpl.GetDentryAt.
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go
index 267dfa83c..8f7bced09 100644
--- a/pkg/sentry/fsimpl/overlay/filesystem.go
+++ b/pkg/sentry/fsimpl/overlay/filesystem.go
@@ -567,7 +567,13 @@ func (fs *filesystem) AccessAt(ctx context.Context, rp *vfs.ResolvingPath, creds
 	if err != nil {
 		return err
 	}
-	return d.checkPermissions(creds, ats)
+	if err := d.checkPermissions(creds, ats); err != nil {
+		return err
+	}
+	if ats.MayWrite() && rp.Mount().ReadOnly() {
+		return linuxerr.EROFS
+	}
+	return nil
 }
 
 // BoundEndpointAt implements vfs.FilesystemImpl.BoundEndpointAt.
diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go
index 5d2c48148..0b093cfb3 100644
--- a/pkg/sentry/fsimpl/tmpfs/filesystem.go
+++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go
@@ -207,7 +207,13 @@ func (fs *filesystem) AccessAt(ctx context.Context, rp *vfs.ResolvingPath, creds
 	if err != nil {
 		return err
 	}
-	return d.inode.checkPermissions(creds, ats)
+	if err := d.inode.checkPermissions(creds, ats); err != nil {
+		return err
+	}
+	if ats.MayWrite() && rp.Mount().ReadOnly() {
+		return linuxerr.EROFS
+	}
+	return nil
 }
 
 // GetDentryAt implements vfs.FilesystemImpl.GetDentryAt.
diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc
index c0c751488..668c0499f 100644
--- a/test/syscalls/linux/mount.cc
+++ b/test/syscalls/linux/mount.cc
@@ -337,6 +337,8 @@ TEST(MountTest, MountReadonly) {
   const struct stat s = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path()));
   EXPECT_EQ(s.st_mode, S_IFDIR | 0777);
 
+  EXPECT_THAT(access(dir.path().c_str(), W_OK), SyscallFailsWithErrno(EROFS));
+
   std::string const filename = JoinPath(dir.path(), "foo");
   EXPECT_THAT(open(filename.c_str(), O_RDWR | O_CREAT, 0777),
               SyscallFailsWithErrno(EROFS));