diff --git a/mount/losetup_linux.go b/mount/losetup_linux.go index a0040dc185a3..88e0497da8d1 100644 --- a/mount/losetup_linux.go +++ b/mount/losetup_linux.go @@ -69,7 +69,10 @@ func getFreeLoopDev() (uint32, error) { return uint32(num), nil } -func setupLoopDev(backingFile, loopDev string, param LoopParams) error { +// setupLoopDev attaches the backing file to the loop device and returns +// the file handle for the loop device. The caller is responsible for +// closing the file handle. +func setupLoopDev(backingFile, loopDev string, param LoopParams) (*os.File, error) { // 1. Open backing file and loop device flags := os.O_RDWR if param.Readonly { @@ -78,19 +81,20 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error { back, err := os.OpenFile(backingFile, flags, 0) if err != nil { - return errors.Wrapf(err, "could not open backing file: %s", backingFile) + return nil, errors.Wrapf(err, "could not open backing file: %s", backingFile) } defer back.Close() + // To make Autoclear (LO_FLAGS_AUTOCLEAR) work, this function intentionally + // doesn't close this file handle on defer. loop, err := os.OpenFile(loopDev, flags, 0) if err != nil { - return errors.Wrapf(err, "could not open loop device: %s", loopDev) + return nil, errors.Wrapf(err, "could not open loop device: %s", loopDev) } - defer loop.Close() // 2. Set FD if _, _, err = ioctl(loop.Fd(), unix.LOOP_SET_FD, back.Fd()); err != nil { - return errors.Wrapf(err, "could not set loop fd for device: %s", loopDev) + return nil, errors.Wrapf(err, "could not set loop fd for device: %s", loopDev) } // 3. Set Info @@ -110,7 +114,7 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error { _, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info))) if err == nil { - return nil + return loop, nil } if param.Direct { @@ -119,13 +123,16 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error { info.Flags &= ^(uint32(unix.LO_FLAGS_DIRECT_IO)) _, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info))) if err == nil { - return nil + return loop, nil } } - // Cleanup loop fd and return error + // Cleanup loop fd and return error. + // If this function doesn't return this file handle, it must close the handle to + // prevent file descriptor leaks. + loop.Close() _, _, _ = ioctl(loop.Fd(), unix.LOOP_CLR_FD, 0) - return errors.Errorf("failed to set loop device info: %v", err) + return nil, errors.Errorf("failed to set loop device info: %v", err) } // setupLoop looks for (and possibly creates) a free loop device, and @@ -142,15 +149,16 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error { // the loop device when done with it. // // Upon success, the file handle to the loop device is returned. -func setupLoop(backingFile string, param LoopParams) (string, error) { +func setupLoop(backingFile string, param LoopParams) (*os.File, error) { for retry := 1; retry < 100; retry++ { num, err := getFreeLoopDev() if err != nil { - return "", err + return nil, err } loopDev := fmt.Sprintf(loopDevFormat, num) - if err := setupLoopDev(backingFile, loopDev, param); err != nil { + file, err := setupLoopDev(backingFile, loopDev, param) + if err != nil { // Per util-linux/sys-utils/losetup.c:create_loop(), // free loop device can race and we end up failing // with EBUSY when trying to set it up. @@ -159,13 +167,13 @@ func setupLoop(backingFile string, param LoopParams) (string, error) { time.Sleep(time.Millisecond * time.Duration(rand.Intn(retry*10))) continue } - return "", err + return nil, err } - return loopDev, nil + return file, nil } - return "", errors.New("timeout creating new loopback device") + return nil, errors.New("timeout creating new loopback device") } func removeLoop(loopdev string) error { @@ -181,7 +189,12 @@ func removeLoop(loopdev string) error { // Attach a specified backing file to a loop device func AttachLoopDevice(backingFile string) (string, error) { - return setupLoop(backingFile, LoopParams{}) + file, err := setupLoop(backingFile, LoopParams{}) + if err != nil { + return "", err + } + defer file.Close() + return file.Name(), nil } // Detach a loop device diff --git a/mount/losetup_linux_test.go b/mount/losetup_linux_test.go index 445bb05d0c14..75bc2b92996d 100644 --- a/mount/losetup_linux_test.go +++ b/mount/losetup_linux_test.go @@ -64,12 +64,13 @@ func TestRoLoop(t *testing.T) { } }() - path, err := setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true}) + file, err := setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true}) if err != nil { t.Fatal(err) } + defer file.Close() - if err := ioutil.WriteFile(path, randomData, os.ModePerm); err == nil { + if _, err := file.Write(randomData); err == nil { t.Fatalf("writing to readonly loop device should fail") } } @@ -84,12 +85,13 @@ func TestRwLoop(t *testing.T) { } }() - path, err := setupLoop(backingFile, LoopParams{Autoclear: true}) + file, err := setupLoop(backingFile, LoopParams{Autoclear: false}) if err != nil { t.Fatal(err) } + defer file.Close() - if err := ioutil.WriteFile(path, randomData, os.ModePerm); err != nil { + if _, err := file.Write(randomData); err != nil { t.Fatal(err) } } diff --git a/mount/mount_linux.go b/mount/mount_linux.go index 7191d40b1707..3f05ebc891ac 100644 --- a/mount/mount_linux.go +++ b/mount/mount_linux.go @@ -79,14 +79,16 @@ func (m *Mount) Mount(target string) (err error) { // or remount with changed data source := m.Source if losetup { - devFile, err := setupLoop(m.Source, LoopParams{ + loFile, err := setupLoop(m.Source, LoopParams{ Readonly: oflags&unix.MS_RDONLY == unix.MS_RDONLY, Autoclear: true}) if err != nil { return err } + defer loFile.Close() + // Mount the loop device instead - source = devFile + source = loFile.Name() } if err := mountAt(chdir, source, target, m.Type, uintptr(oflags), data); err != nil { return err