Skip to content

Commit ddf6594

Browse files
dmcgowandmcgowan
authored
Merge pull request #5076 from AkihiroSuda/ovl-k511
overlay: support "userxattr" option (kernel 5.11)
2 parents a5d17eb + 9ade247 commit ddf6594

3 files changed

Lines changed: 124 additions & 6 deletions

File tree

‎snapshots/overlay/check.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/containerd/containerd/log"
2828
"github.com/containerd/containerd/mount"
29+
"github.com/containerd/containerd/sys"
2930
"github.com/containerd/continuity/fs"
3031
"github.com/pkg/errors"
3132
)
@@ -86,3 +87,84 @@ func Supported(root string) error {
8687
}
8788
return supportsMultipleLowerDir(root)
8889
}
90+
91+
// NeedsUserXAttr returns whether overlayfs should be mounted with the "userxattr" mount option.
92+
//
93+
// The "userxattr" option is needed for mounting overlayfs inside a user namespace with kernel >= 5.11.
94+
//
95+
// The "userxattr" option is NOT needed for the initial user namespace (aka "the host").
96+
//
97+
// Also, Ubuntu (since circa 2015) and Debian (since 10) with kernel < 5.11 can mount
98+
// the overlayfs in a user namespace without the "userxattr" option.
99+
//
100+
// The corresponding kernel commit: https://github.com/torvalds/linux/commit/2d2f2d7322ff43e0fe92bf8cccdc0b09449bf2e1
101+
// > ovl: user xattr
102+
// >
103+
// > Optionally allow using "user.overlay." namespace instead of "trusted.overlay."
104+
// > ...
105+
// > Disable redirect_dir and metacopy options, because these would allow privilege escalation through direct manipulation of the
106+
// > "user.overlay.redirect" or "user.overlay.metacopy" xattrs.
107+
// > ...
108+
//
109+
// The "userxattr" support is not exposed in "/sys/module/overlay/parameters".
110+
func NeedsUserXAttr(d string) (bool, error) {
111+
if !sys.RunningInUserNS() {
112+
// we are the real root (i.e., the root in the initial user NS),
113+
// so we do never need "userxattr" opt.
114+
return false, nil
115+
}
116+
117+
// TODO: add fast path for kernel >= 5.11 .
118+
//
119+
// Keep in mind that distro vendors might be going to backport the patch to older kernels.
120+
// So we can't completely remove the check.
121+
122+
tdRoot := filepath.Join(d, "userxattr-check")
123+
if err := os.RemoveAll(tdRoot); err != nil {
124+
log.L.WithError(err).Warnf("Failed to remove check directory %v", tdRoot)
125+
}
126+
127+
if err := os.MkdirAll(tdRoot, 0700); err != nil {
128+
return false, err
129+
}
130+
131+
defer func() {
132+
if err := os.RemoveAll(tdRoot); err != nil {
133+
log.L.WithError(err).Warnf("Failed to remove check directory %v", tdRoot)
134+
}
135+
}()
136+
137+
td, err := ioutil.TempDir(tdRoot, "")
138+
if err != nil {
139+
return false, err
140+
}
141+
142+
for _, dir := range []string{"lower1", "lower2", "upper", "work", "merged"} {
143+
if err := os.Mkdir(filepath.Join(td, dir), 0755); err != nil {
144+
return false, err
145+
}
146+
}
147+
148+
opts := []string{
149+
fmt.Sprintf("lowerdir=%s:%s,upperdir=%s,workdir=%s", filepath.Join(td, "lower2"), filepath.Join(td, "lower1"), filepath.Join(td, "upper"), filepath.Join(td, "work")),
150+
"userxattr",
151+
}
152+
153+
m := mount.Mount{
154+
Type: "overlay",
155+
Source: "overlay",
156+
Options: opts,
157+
}
158+
159+
dest := filepath.Join(td, "merged")
160+
if err := m.Mount(dest); err != nil {
161+
// Probably the host is running Ubuntu/Debian kernel (< 5.11) with the userns patch but without the userxattr patch.
162+
// Return false without error.
163+
log.L.WithError(err).Debugf("cannot mount overlay with \"userxattr\", probably the kernel does not support userxattr")
164+
return false, nil
165+
}
166+
if err := mount.UnmountAll(dest, 0); err != nil {
167+
log.L.WithError(err).Warnf("Failed to unmount check directory %v", dest)
168+
}
169+
return true, nil
170+
}

‎snapshots/overlay/overlay.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/containerd/containerd/snapshots/storage"
3434
"github.com/containerd/continuity/fs"
3535
"github.com/pkg/errors"
36+
"github.com/sirupsen/logrus"
3637
)
3738

3839
// SnapshotterConfig is used to configure the overlay snapshotter instance
@@ -57,6 +58,7 @@ type snapshotter struct {
5758
ms *storage.MetaStore
5859
asyncRemove bool
5960
indexOff bool
61+
userxattr bool // whether to enable "userxattr" mount option
6062
}
6163

6264
// NewSnapshotter returns a Snapshotter which uses overlayfs. The overlayfs
@@ -95,11 +97,18 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) {
9597
indexOff = true
9698
}
9799

100+
// figure out whether "userxattr" option is recognized by the kernel && needed
101+
userxattr, err := NeedsUserXAttr(root)
102+
if err != nil {
103+
logrus.WithError(err).Warnf("cannot detect whether \"userxattr\" option needs to be used, assuming to be %v", userxattr)
104+
}
105+
98106
return &snapshotter{
99107
root: root,
100108
ms: ms,
101109
asyncRemove: config.asyncRemove,
102110
indexOff: indexOff,
111+
userxattr: userxattr,
103112
}, nil
104113
}
105114

@@ -464,6 +473,10 @@ func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount {
464473
options = append(options, "index=off")
465474
}
466475

476+
if o.userxattr {
477+
options = append(options, "userxattr")
478+
}
479+
467480
if s.Kind == snapshots.KindActive {
468481
options = append(options,
469482
fmt.Sprintf("workdir=%s", o.workPath(s.ID)),

‎snapshots/overlay/overlay_test.go‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,21 @@ func testOverlayOverlayMount(t *testing.T, newSnapshotter testsuite.SnapshotterF
173173
upper = "upperdir=" + filepath.Join(bp, "fs")
174174
lower = "lowerdir=" + getParents(ctx, o, root, "/tmp/layer2")[0]
175175
)
176-
for i, v := range []string{
176+
177+
expected := []string{
177178
"index=off",
179+
}
180+
if userxattr, err := NeedsUserXAttr(root); err != nil {
181+
t.Fatal(err)
182+
} else if userxattr {
183+
expected = append(expected, "userxattr")
184+
}
185+
expected = append(expected, []string{
178186
work,
179187
upper,
180188
lower,
181-
} {
189+
}...)
190+
for i, v := range expected {
182191
if m.Options[i] != v {
183192
t.Errorf("expected %q but received %q", v, m.Options[i])
184193
}
@@ -335,12 +344,26 @@ func testOverlayView(t *testing.T, newSnapshotter testsuite.SnapshotterFunc) {
335344
if m.Source != "overlay" {
336345
t.Errorf("mount source should be overlay but received %q", m.Source)
337346
}
338-
if len(m.Options) != 2 {
339-
t.Errorf("expected 1 additional mount option but got %d", len(m.Options))
347+
348+
expectedOptions := 2
349+
userxattr, err := NeedsUserXAttr(root)
350+
if err != nil {
351+
t.Fatal(err)
352+
}
353+
if userxattr {
354+
expectedOptions++
355+
}
356+
357+
if len(m.Options) != expectedOptions {
358+
t.Errorf("expected %d additional mount option but got %d", expectedOptions, len(m.Options))
340359
}
341360
lowers := getParents(ctx, o, root, "/tmp/view2")
342361
expected = fmt.Sprintf("lowerdir=%s:%s", lowers[0], lowers[1])
343-
if m.Options[1] != expected {
344-
t.Errorf("expected option %q but received %q", expected, m.Options[0])
362+
optIdx := 1
363+
if userxattr {
364+
optIdx++
365+
}
366+
if m.Options[optIdx] != expected {
367+
t.Errorf("expected option %q but received %q", expected, m.Options[optIdx])
345368
}
346369
}

0 commit comments

Comments
 (0)