From 1bbdbfcfe2399dbc38bc9eb0dc706ee2e7543f27 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 26 Jun 2025 15:52:26 -0700 Subject: [PATCH] Check MS_RDONLY for O_TRUNC before doing file truncation. PiperOrigin-RevId: 776313066 --- pkg/sentry/fsimpl/gofer/filesystem.go | 6 +++++- pkg/sentry/fsimpl/kernfs/filesystem.go | 24 ++++++++++++++++++++++-- pkg/sentry/fsimpl/kernfs/kernfs.go | 17 +++++++++++++---- pkg/sentry/fsimpl/tmpfs/filesystem.go | 10 ++++++++-- test/syscalls/linux/open.cc | 1 + 5 files changed, 49 insertions(+), 9 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 2f5e905607..1000af65bd 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -1079,8 +1079,13 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, opts *vfs.Open defer d.fs.renameMu.RUnlock() } + mnt := rp.Mount() trunc := opts.Flags&linux.O_TRUNC != 0 && d.fileType() == linux.S_IFREG if trunc { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, err + } + defer mnt.EndWrite() // Lock metadataMu *while* we open a regular file with O_TRUNC because // open(2) will change the file size on server. d.metadataMu.Lock() @@ -1089,7 +1094,6 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, opts *vfs.Open var vfd *vfs.FileDescription var err error - mnt := rp.Mount() switch d.fileType() { case linux.S_IFREG: if !d.fs.opts.regularFilesUseSpecialFileFD { diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index b3b045bf06..e34d5ea284 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -512,6 +512,8 @@ func (fs *Filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v // OpenAt implements vfs.FilesystemImpl.OpenAt. func (fs *Filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.OpenOptions) (*vfs.FileDescription, error) { ats := vfs.AccessTypesForOpenFlags(&opts) + trunc := opts.Flags&linux.O_TRUNC != 0 + mnt := rp.Mount() // Do not create new file. if opts.Flags&linux.O_CREAT == 0 { @@ -526,6 +528,12 @@ func (fs *Filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf fs.mu.RUnlock() return nil, err } + if trunc && d.isRegular() { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, err + } + defer mnt.EndWrite() + } // Open may block so we need to unlock fs.mu. IncRef d to prevent // its destruction while fs.mu is unlocked. d.IncRef() @@ -561,6 +569,12 @@ func (fs *Filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf if err := start.inode.CheckPermissions(ctx, rp.Credentials(), ats); err != nil { return nil, err } + if trunc && start.isRegular() { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, err + } + defer mnt.EndWrite() + } // Open may block so we need to unlock fs.mu. IncRef d to prevent // its destruction while fs.mu is unlocked. start.IncRef() @@ -612,10 +626,10 @@ afterTrailingSymlink: if err := parent.inode.CheckPermissions(ctx, rp.Credentials(), vfs.MayWrite); err != nil { return nil, err } - if err := rp.Mount().CheckBeginWrite(); err != nil { + if err := mnt.CheckBeginWrite(); err != nil { return nil, err } - defer rp.Mount().EndWrite() + defer mnt.EndWrite() // Create and open the child. childI, err := parent.inode.NewFile(ctx, pc, opts) if err != nil { @@ -646,6 +660,12 @@ afterTrailingSymlink: if err := child.inode.CheckPermissions(ctx, rp.Credentials(), ats); err != nil { return nil, err } + if trunc && child.isRegular() { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, err + } + defer mnt.EndWrite() + } if child.isDir() { // Can't open directories with O_CREAT. if opts.Flags&linux.O_CREAT != 0 { diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index d7d600626e..32249590c1 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -202,6 +202,9 @@ const ( // Dentry points to a symlink inode. dflagsIsSymlink + + // Dentry points to a regular file inode. + dflagsIsRegular ) // Dentry implements vfs.DentryImpl. @@ -519,12 +522,13 @@ func (d *Dentry) Init(fs *Filesystem, inode Inode) { d.fs = fs d.inode = inode d.refs.Store(1) - ftype := inode.Mode().FileType() - if ftype == linux.ModeDirectory { + switch inode.Mode().FileType() { + case linux.ModeDirectory: d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsDir) - } - if ftype == linux.ModeSymlink { + case linux.ModeSymlink: d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsSymlink) + case linux.ModeRegular: + d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsRegular) } refs.Register(d) inode.RegisterDentry(d) @@ -553,6 +557,11 @@ func (d *Dentry) isSymlink() bool { return d.flags.Load()&dflagsIsSymlink != 0 } +// isRegular checks whether the dentry points to a regular file inode. +func (d *Dentry) isRegular() bool { + return d.flags.Load()&dflagsIsRegular != 0 +} + // InotifyWithParent implements vfs.DentryImpl.InotifyWithParent. func (d *Dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, et vfs.EventType) { if d.isDir() { diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index a1641c265f..31abbebc7f 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -459,11 +459,17 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, opts *vfs.Open case *regularFile: var fd regularFileFD fd.LockFD.Init(&d.inode.locks) - if err := fd.vfsfd.Init(&fd, opts.Flags, rp.Mount(), &d.vfsd, &vfs.FileDescriptionOptions{AllowDirectIO: true}); err != nil { + mnt := rp.Mount() + if err := fd.vfsfd.Init(&fd, opts.Flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{AllowDirectIO: true}); err != nil { return nil, err } if !afterCreate && opts.Flags&linux.O_TRUNC != 0 { - if _, err := impl.truncate(0); err != nil { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, err + } + _, err := impl.truncate(0) + mnt.EndWrite() + if err != nil { return nil, err } } diff --git a/test/syscalls/linux/open.cc b/test/syscalls/linux/open.cc index 8a21c26575..e166cd9041 100644 --- a/test/syscalls/linux/open.cc +++ b/test/syscalls/linux/open.cc @@ -436,6 +436,7 @@ TEST_F(OpenTest, CanTruncateReadOnly) { struct stat stat; EXPECT_THAT(fstat(fd1.get(), &stat), SyscallSucceeds()); EXPECT_EQ(stat.st_size, 0); + EXPECT_THAT(write(fd1.get(), "x", 1), SyscallFailsWithErrno(EBADF)); } // If we don't have read permission on the file, opening with