Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

AkihiroSuda
Copy link
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 <akihiro.suda.cz@hco.ntt.co.jp>
(cherry picked from commit 62b5194)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda added this to the 20.10.6 milestone Apr 6, 2021
Copy link
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
Member

@tonistiigi @cpuguy83 ptal

Copy link
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
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