From 5ce542ecc749cb9a1e8d216c7181aeaebfbc3110 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Mon, 7 Jan 2019 23:01:10 -0800 Subject: Undo changes in case of failure to create file/dir/symlink File/dir/symlink creation is multi-step and may leave state behind in case of failure in one of the steps. Added best effort attempt to clean up. PiperOrigin-RevId: 228286612 Change-Id: Ib03c27cd3d3e4f44d0352edc6ee212a53412d7f1 --- runsc/fsgofer/BUILD | 1 + runsc/fsgofer/fsgofer.go | 36 +++++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) (limited to 'runsc/fsgofer') diff --git a/runsc/fsgofer/BUILD b/runsc/fsgofer/BUILD index ab12388ab..756c20ad7 100644 --- a/runsc/fsgofer/BUILD +++ b/runsc/fsgofer/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/log", "//pkg/p9", "//pkg/syserr", + "//runsc/specutils", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index b5746447f..9955d0750 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -35,6 +35,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/fd" "gvisor.googlesource.com/gvisor/pkg/log" "gvisor.googlesource.com/gvisor/pkg/p9" + "gvisor.googlesource.com/gvisor/runsc/specutils" ) const ( @@ -379,19 +380,20 @@ func (l *localFile) Create(name string, mode p9.OpenFlags, perm p9.FileMode, uid if err != nil { return nil, nil, p9.QID{}, 0, extractErrno(err) } - if err := fchown(fd, uid, gid); err != nil { + cu := specutils.MakeCleanup(func() { syscall.Close(fd) - if e := syscall.Unlinkat(l.controlFD(), name); e != nil { - log.Warningf("error unlinking file %q after failed chown: %v", name, e) + // Best effort attempt to remove the file in case of failure. + if err := syscall.Unlinkat(l.controlFD(), name); err != nil { + log.Warningf("error unlinking file %q after failure: %v", path.Join(l.hostPath, name), err) } + }) + defer cu.Clean() + + if err := fchown(fd, uid, gid); err != nil { return nil, nil, p9.QID{}, 0, extractErrno(err) } stat, err := stat(fd) if err != nil { - syscall.Close(fd) - if e := syscall.Unlinkat(l.controlFD(), name); e != nil { - log.Warningf("error unlinking file %q after failed stat: %v", name, e) - } return nil, nil, p9.QID{}, 0, extractErrno(err) } @@ -404,6 +406,8 @@ func (l *localFile) Create(name string, mode p9.OpenFlags, perm p9.FileMode, uid openedFile: f, mode: mode, } + + cu.Release() return newFDMaybe(c.openedFile), c, l.attachPoint.makeQID(stat), 0, nil } @@ -420,6 +424,13 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID) if err := syscall.Mkdirat(l.controlFD(), name, uint32(perm.Permissions())); err != nil { return p9.QID{}, extractErrno(err) } + cu := specutils.MakeCleanup(func() { + // Best effort attempt to remove the dir in case of failure. + if err := unix.Unlinkat(l.controlFD(), name, unix.AT_REMOVEDIR); err != nil { + log.Warningf("error unlinking dir %q after failure: %v", path.Join(l.hostPath, name), err) + } + }) + defer cu.Clean() // Open directory to change ownership and stat it. flags := syscall.O_DIRECTORY | syscall.O_RDONLY | openFlags @@ -436,6 +447,8 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID) if err != nil { return p9.QID{}, extractErrno(err) } + + cu.Release() return l.attachPoint.makeQID(stat), nil } @@ -759,6 +772,13 @@ func (l *localFile) Symlink(target, newName string, uid p9.UID, gid p9.GID) (p9. if err := unix.Symlinkat(target, l.controlFD(), newName); err != nil { return p9.QID{}, extractErrno(err) } + cu := specutils.MakeCleanup(func() { + // Best effort attempt to remove the symlink in case of failure. + if err := syscall.Unlinkat(l.controlFD(), newName); err != nil { + log.Warningf("error unlinking file %q after failure: %v", path.Join(l.hostPath, newName), err) + } + }) + defer cu.Clean() // Open symlink to change ownership and stat it. fd, err := syscall.Openat(l.controlFD(), newName, unix.O_PATH|openFlags, 0) @@ -774,6 +794,8 @@ func (l *localFile) Symlink(target, newName string, uid p9.UID, gid p9.GID) (p9. if err != nil { return p9.QID{}, extractErrno(err) } + + cu.Release() return l.attachPoint.makeQID(stat), nil } -- cgit v1.2.3