Skip to content

Commit 05a2e28

Browse files
author
Kazuyoshi Kato
committed
mount: make setupLoop() work with with Autoclear
setupLoop()'s Autoclear (LO_FLAGS_AUTOCLEAR) will destruct the loopback device when all associated file descriptors are closed. However this behavior didn't work before since setupLoop() was returning a file name. The looppack device was destructed at the end of the function when LoopParams had Autoclear = true. Fixes containerd#4969. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
1 parent ccde82d commit 05a2e28

File tree

3 files changed

+39
-22
lines changed

3 files changed

+39
-22
lines changed

‎mount/losetup_linux.go‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ func getFreeLoopDev() (uint32, error) {
6969
return uint32(num), nil
7070
}
7171

72-
func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
72+
// setupLoopDev attaches the backing file to the loop device and returns
73+
// the file handle for the loop device. The caller is responsible for
74+
// closing the file handle.
75+
func setupLoopDev(backingFile, loopDev string, param LoopParams) (*os.File, error) {
7376
// 1. Open backing file and loop device
7477
flags := os.O_RDWR
7578
if param.Readonly {
@@ -78,19 +81,20 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
7881

7982
back, err := os.OpenFile(backingFile, flags, 0)
8083
if err != nil {
81-
return errors.Wrapf(err, "could not open backing file: %s", backingFile)
84+
return nil, errors.Wrapf(err, "could not open backing file: %s", backingFile)
8285
}
8386
defer back.Close()
8487

88+
// To make Autoclear (LO_FLAGS_AUTOCLEAR) work, this function intentionally
89+
// doesn't close this file handle on defer.
8590
loop, err := os.OpenFile(loopDev, flags, 0)
8691
if err != nil {
87-
return errors.Wrapf(err, "could not open loop device: %s", loopDev)
92+
return nil, errors.Wrapf(err, "could not open loop device: %s", loopDev)
8893
}
89-
defer loop.Close()
9094

9195
// 2. Set FD
9296
if _, _, err = ioctl(loop.Fd(), unix.LOOP_SET_FD, back.Fd()); err != nil {
93-
return errors.Wrapf(err, "could not set loop fd for device: %s", loopDev)
97+
return nil, errors.Wrapf(err, "could not set loop fd for device: %s", loopDev)
9498
}
9599

96100
// 3. Set Info
@@ -110,7 +114,7 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
110114

111115
_, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info)))
112116
if err == nil {
113-
return nil
117+
return loop, nil
114118
}
115119

116120
if param.Direct {
@@ -119,13 +123,16 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
119123
info.Flags &= ^(uint32(unix.LO_FLAGS_DIRECT_IO))
120124
_, _, err = ioctl(loop.Fd(), unix.LOOP_SET_STATUS64, uintptr(unsafe.Pointer(&info)))
121125
if err == nil {
122-
return nil
126+
return loop, nil
123127
}
124128
}
125129

126-
// Cleanup loop fd and return error
130+
// Cleanup loop fd and return error.
131+
// If this function doesn't return this file handle, it must close the handle to
132+
// prevent file descriptor leaks.
133+
loop.Close()
127134
_, _, _ = ioctl(loop.Fd(), unix.LOOP_CLR_FD, 0)
128-
return errors.Errorf("failed to set loop device info: %v", err)
135+
return nil, errors.Errorf("failed to set loop device info: %v", err)
129136
}
130137

131138
// setupLoop looks for (and possibly creates) a free loop device, and
@@ -142,15 +149,16 @@ func setupLoopDev(backingFile, loopDev string, param LoopParams) error {
142149
// the loop device when done with it.
143150
//
144151
// Upon success, the file handle to the loop device is returned.
145-
func setupLoop(backingFile string, param LoopParams) (string, error) {
152+
func setupLoop(backingFile string, param LoopParams) (*os.File, error) {
146153
for retry := 1; retry < 100; retry++ {
147154
num, err := getFreeLoopDev()
148155
if err != nil {
149-
return "", err
156+
return nil, err
150157
}
151158

152159
loopDev := fmt.Sprintf(loopDevFormat, num)
153-
if err := setupLoopDev(backingFile, loopDev, param); err != nil {
160+
file, err := setupLoopDev(backingFile, loopDev, param)
161+
if err != nil {
154162
// Per util-linux/sys-utils/losetup.c:create_loop(),
155163
// free loop device can race and we end up failing
156164
// with EBUSY when trying to set it up.
@@ -159,13 +167,13 @@ func setupLoop(backingFile string, param LoopParams) (string, error) {
159167
time.Sleep(time.Millisecond * time.Duration(rand.Intn(retry*10)))
160168
continue
161169
}
162-
return "", err
170+
return nil, err
163171
}
164172

165-
return loopDev, nil
173+
return file, nil
166174
}
167175

168-
return "", errors.New("timeout creating new loopback device")
176+
return nil, errors.New("timeout creating new loopback device")
169177
}
170178

171179
func removeLoop(loopdev string) error {
@@ -181,7 +189,12 @@ func removeLoop(loopdev string) error {
181189

182190
// Attach a specified backing file to a loop device
183191
func AttachLoopDevice(backingFile string) (string, error) {
184-
return setupLoop(backingFile, LoopParams{})
192+
file, err := setupLoop(backingFile, LoopParams{})
193+
if err != nil {
194+
return "", err
195+
}
196+
defer file.Close()
197+
return file.Name(), nil
185198
}
186199

187200
// Detach a loop device

‎mount/losetup_linux_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,13 @@ func TestRoLoop(t *testing.T) {
6464
}
6565
}()
6666

67-
path, err := setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true})
67+
file, err := setupLoop(backingFile, LoopParams{Readonly: true, Autoclear: true})
6868
if err != nil {
6969
t.Fatal(err)
7070
}
71+
defer file.Close()
7172

72-
if err := ioutil.WriteFile(path, randomData, os.ModePerm); err == nil {
73+
if _, err := file.Write(randomData); err == nil {
7374
t.Fatalf("writing to readonly loop device should fail")
7475
}
7576
}
@@ -84,12 +85,13 @@ func TestRwLoop(t *testing.T) {
8485
}
8586
}()
8687

87-
path, err := setupLoop(backingFile, LoopParams{Autoclear: true})
88+
file, err := setupLoop(backingFile, LoopParams{Autoclear: false})
8889
if err != nil {
8990
t.Fatal(err)
9091
}
92+
defer file.Close()
9193

92-
if err := ioutil.WriteFile(path, randomData, os.ModePerm); err != nil {
94+
if _, err := file.Write(randomData); err != nil {
9395
t.Fatal(err)
9496
}
9597
}

‎mount/mount_linux.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,16 @@ func (m *Mount) Mount(target string) (err error) {
7979
// or remount with changed data
8080
source := m.Source
8181
if losetup {
82-
devFile, err := setupLoop(m.Source, LoopParams{
82+
loFile, err := setupLoop(m.Source, LoopParams{
8383
Readonly: oflags&unix.MS_RDONLY == unix.MS_RDONLY,
8484
Autoclear: true})
8585
if err != nil {
8686
return err
8787
}
88+
defer loFile.Close()
89+
8890
// Mount the loop device instead
89-
source = devFile
91+
source = loFile.Name()
9092
}
9193
if err := mountAt(chdir, source, target, m.Type, uintptr(oflags), data); err != nil {
9294
return err

0 commit comments

Comments
 (0)