Skip to content

Check MS_RDONLY for O_TRUNC before doing file truncation. #11870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pkg/sentry/fsimpl/gofer/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
24 changes: 22 additions & 2 deletions pkg/sentry/fsimpl/kernfs/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 13 additions & 4 deletions pkg/sentry/fsimpl/kernfs/kernfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ const (

// Dentry points to a symlink inode.
dflagsIsSymlink

// Dentry points to a regular file inode.
dflagsIsRegular
)

// Dentry implements vfs.DentryImpl.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down
10 changes: 8 additions & 2 deletions pkg/sentry/fsimpl/tmpfs/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
1 change: 1 addition & 0 deletions test/syscalls/linux/open.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down