Skip to content

[20.10 backport] btrfs: Allow unprivileged user to delete subvolumes (kernel >= 4.18)#42253

Merged
tiborvass merged 1 commit intomoby:20.10from
AkihiroSuda:btrfs-allow-unprivileged-20.10
Apr 7, 2021
Merged

[20.10 backport] btrfs: Allow unprivileged user to delete subvolumes (kernel >= 4.18)#42253
tiborvass merged 1 commit intomoby:20.10from
AkihiroSuda:btrfs-allow-unprivileged-20.10

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

Cherry-pick #42203


- What I did

Fix #41762 ([rootless] driver "btrfs" failed to remove root filesystem: Failed to destroy btrfs snapshot /home/<USER>/.local/share/docker/btrfs/subvolumes for <ID>: operation not permitted)

- How I did it

Cherry-pick "drivers: btrfs: Allow unprivileged user to delete subvolumes" from containers/storage
containers/storage@831e32b

In btrfs, subvolume can be deleted by IOC_SNAP_DESTROY ioctl but there
is one catch: unprivileged IOC_SNAP_DESTROY call is restricted by default.

This is because IOC_SNAP_DESTROY only performs permission checks on
the top directory(subvolume) and unprivileged user might delete dirs/files
which cannot be deleted otherwise. This restriction can be relaxed if
user_subvol_rm_allowed mount option is used.

Although the above ioctl had been the only way to delete a subvolume,
btrfs now allows deletion of subvolume just like regular directory
(i.e. rmdir sycall) since kernel 4.18.

So if we fail to cleanup subvolume in subvolDelete(), just fallback to
system.EnsureRmoveall() to try to cleanup subvolumes again.
(Note: quota needs privilege, so if quota is enabled we do not fallback)

This fix will allow non-privileged container works with btrfs backend.

- How to verify it

$ dockerd-rootless.sh --data-root /mnt/btrfs/varlibdocker
$ docker --context=rootless run --rm hello-world

- Description for the changelog

btrfs: Allow unprivileged user to delete subvolumes (kernel >= 4.18)

- A picture of a cute animal (not mandatory but encouraged)
🐧

Fix issue 41762

Cherry-pick "drivers: btrfs: Allow unprivileged user to delete subvolumes" from containers/storage
containers/storage@831e32b

> In btrfs, subvolume can be deleted by IOC_SNAP_DESTROY ioctl but there
> is one catch: unprivileged IOC_SNAP_DESTROY call is restricted by default.
>
> This is because IOC_SNAP_DESTROY only performs permission checks on
> the top directory(subvolume) and unprivileged user might delete dirs/files
> which cannot be deleted otherwise. This restriction can be relaxed if
> user_subvol_rm_allowed mount option is used.
>
> Although the above ioctl had been the only way to delete a subvolume,
> btrfs now allows deletion of subvolume just like regular directory
> (i.e. rmdir sycall) since kernel 4.18.
>
> So if we fail to cleanup subvolume in subvolDelete(), just fallback to
> system.EnsureRmoveall() to try to cleanup subvolumes again.
> (Note: quota needs privilege, so if quota is enabled we do not fallback)
>
> This fix will allow non-privileged container works with btrfs backend.

Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit 62b5194)
Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda added this to the 20.10.6 milestone Apr 6, 2021
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

@tonistiigi @cpuguy83 ptal

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

This one also started to become more flaky recently;

--- FAIL: TestRestartDaemonWithRestartingContainer (2.31s)
    daemon_linux_test.go:224: [d32ee3bb7c25b] failed to start daemon with arguments [--data-root /go/src/github.com/docker/docker/bundles/test-integration/TestRestartDaemonWithRestartingContainer/d32ee3bb7c25b/root --exec-root /tmp/dxr/d32ee3bb7c25b --pidfile /go/src/github.com/docker/docker/bundles/test-integration/TestRestartDaemonWithRestartingContainer/d32ee3bb7c25b/docker.pid --userland-proxy=true --containerd-namespace d32ee3bb7c25b --containerd-plugins-namespace d32ee3bb7c25bp --containerd /var/run/docker/containerd/containerd.sock --host unix:///tmp/docker-integration/d32ee3bb7c25b.sock --debug --storage-driver overlay2] : [d32ee3bb7c25b] daemon exited during startup: exit status 1
    panic.go:563: [d32ee3bb7c25b] daemon is not started

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants