From 0d7a1d07110f54a7f9740150d873f9bd28c2d43d Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Sat, 6 Aug 2022 19:58:54 -0700
Subject: [PATCH] Lower layer files that can not be copied up are not writable.

Make access(W_OK) return EACCES in overlayfs if file exists only
on lower layer and can not be copied up. Earlier, access(W_OK)
was succeeding but operations that cause copy-up (like
open(WR_ONLY)) were failing. This is likely to confuse
applications.

PiperOrigin-RevId: 465818314
---
 pkg/sentry/fsimpl/overlay/copy_up.go    | 20 ++++++++++++++------
 pkg/sentry/fsimpl/overlay/filesystem.go | 11 ++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go
index 240319646..f073b2369 100644
--- a/pkg/sentry/fsimpl/overlay/copy_up.go
+++ b/pkg/sentry/fsimpl/overlay/copy_up.go
@@ -31,6 +31,18 @@ func (d *dentry) isCopiedUp() bool {
 	return d.copiedUp.Load() != 0
 }
 
+func (d *dentry) canBeCopiedUp() bool {
+	ftype := d.mode.Load() & linux.S_IFMT
+	switch ftype {
+	case linux.S_IFREG, linux.S_IFDIR, linux.S_IFLNK, linux.S_IFBLK, linux.S_IFCHR:
+		// Can be copied-up.
+		return true
+	default:
+		// Can't be copied-up.
+		return false
+	}
+}
+
 // copyUpLocked ensures that d exists on the upper layer, i.e. d.upperVD.Ok().
 //
 // Preconditions: filesystem.renameMu must be locked.
@@ -48,12 +60,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy
 	// credentials from context rather an take an explicit creds parameter.
 	ctx = auth.ContextWithCredentials(ctx, d.fs.creds)
 
-	ftype := d.mode.Load() & linux.S_IFMT
-	switch ftype {
-	case linux.S_IFREG, linux.S_IFDIR, linux.S_IFLNK, linux.S_IFBLK, linux.S_IFCHR:
-		// Can be copied-up.
-	default:
-		// Can't be copied-up.
+	if !d.canBeCopiedUp() {
 		return linuxerr.EPERM
 	}
 
@@ -92,6 +99,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy
 	}
 
 	// Perform copy-up.
+	ftype := d.mode.Load() & linux.S_IFMT
 	newpop := vfs.PathOperation{
 		Root:  d.parent.upperVD,
 		Start: d.parent.upperVD,
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go
index 0bb8b439a..b53fa6b54 100644
--- a/pkg/sentry/fsimpl/overlay/filesystem.go
+++ b/pkg/sentry/fsimpl/overlay/filesystem.go
@@ -585,9 +585,18 @@ func (fs *filesystem) AccessAt(ctx context.Context, rp *vfs.ResolvingPath, creds
 	if err := d.checkPermissions(creds, ats); err != nil {
 		return err
 	}
-	if ats.MayWrite() && rp.Mount().ReadOnly() {
+	if !ats.MayWrite() {
+		// Not requesting write permission.  Allow it.
+		return nil
+	}
+	if rp.Mount().ReadOnly() {
 		return linuxerr.EROFS
 	}
+	if !d.upperVD.Ok() && !d.canBeCopiedUp() {
+		// A lower layer file that can not be copied up, can not be written to.
+		// Error out here. Don't give the application false hopes.
+		return linuxerr.EACCES
+	}
 	return nil
 }