Skip to content
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

[v1.1.8 regression] Sticky bit on container tmpfs no longer set #3952

Closed
ohookins opened this issue Aug 1, 2023 · 7 comments · Fixed by #3956
Closed

[v1.1.8 regression] Sticky bit on container tmpfs no longer set #3952

ohookins opened this issue Aug 1, 2023 · 7 comments · Fixed by #3956

Comments

@ohookins
Copy link

ohookins commented Aug 1, 2023

Description

On runc 1.1.7 our containers had a tmpfs mounted in, which as expected had the sticky bit set:

app@d28c3e42b1a4:~$ ls -lah / | grep tmp
drwxrwxrwt   2 root root   40 Aug  1 05:09 tmp

Upon upgrading to runc 1.1.8 (via containerd 1.6.22) it seems that the sticky bit is no longer set on the tmpfs:

app@3aab0377e072:~$ ls -alh / | grep tmp
drwxrwxrwx   2 root root   40 Aug  1 04:43 tmp

At a glance, this seems to be a regression introduced in #3916

Steps to reproduce the issue

  1. Upgrade to runc 1.1.8
  2. Run a container with the following configuration snippet:
            "Tmpfs": {
                "/tmp": "size=1073741824,defaults,noexec"
            },
  1. Observe that /tmp does not have the usual sticky bit set, whereas it did in runc 1.1.7.

Describe the results you received and expected

/tmp sticky bit is not set, but previously it was.

What version of runc are you using?

1.1.8

Host OS information

NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

Host kernel information

Linux xxxx 5.15.0-1039-aws #44~20.04.1-Ubuntu SMP Thu Jun 22 12:21:12 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

@lifubang
Copy link
Member

lifubang commented Aug 1, 2023

For runc v1.1.8, when you run a container for the first time, the /tmp directory has a sticky bit, if you reuse this rootfs to run a new container, there is no sticky bit now. It is the same in v1.1.7 and other versions of runc.
So, could you please check that you reuse the container's rootfs when you test v1.1.7 and v1.1.8.
Because for the second time you run a new container, the dest directory /tmp has existed in the rootfs of the container, and its mode is 0o755, which has no sticky bit.

@AkihiroSuda AkihiroSuda changed the title Sticky bit on container tmpfs no longer set [v1.1.8 regression] Sticky bit on container tmpfs no longer set Aug 1, 2023
@ohookins
Copy link
Author

ohookins commented Aug 2, 2023

@lifubang there is no reuse of containers, they are being created from scratch. Let me provide a bit more detail as I've been able to reproduce locally outside of our production cluster (via Colima):

colima:/usr/bin# ls -alh runc*
lrwxrwxrwx    1 root     root          16 Aug  2 00:02 runc -> runc.amd64-1.1.7
-rwxr-xr-x    1 root     root        9.2M Aug  1 23:57 runc.amd64-1.1.7
-rwxr-xr-x    1 root     root       10.2M Aug  1 23:57 runc.amd64-1.1.8

I run a simple container to test via Docker (which in turn is using containerd shim):

$ docker run --rm -ti --tmpfs /tmp ubuntu:22.04
root@ac8dfe5ce680:/# ls -alh / | grep tmp
drwxrwxrwt   2 root root   40 Aug  2 00:03 tmp
root@ac8dfe5ce680:/#

I then change to use runc 1.1.8:

colima:/usr/bin# rm runc
colima:/usr/bin# ln -s runc.amd64-1.1.8 runc
colima:/usr/bin# ls -alh runc*
lrwxrwxrwx    1 root     root          16 Aug  2 00:03 runc -> runc.amd64-1.1.8
-rwxr-xr-x    1 root     root        9.2M Aug  1 23:57 runc.amd64-1.1.7
-rwxr-xr-x    1 root     root       10.2M Aug  1 23:57 runc.amd64-1.1.8

And perform the same test:

$ docker run --rm -ti --tmpfs /tmp ubuntu:22.04
root@09611659b1ac:/# ls -alh / | grep tmp
drwxrwxrwx   2 root root   40 Aug  2 00:04 tmp
root@09611659b1ac:/#

@kolyshkin
Copy link
Contributor

@cpuguy83 @neersighted 🙏 PTAL

@kolyshkin kolyshkin added this to the 1.1.9 milestone Aug 2, 2023
@lifubang
Copy link
Member

lifubang commented Aug 2, 2023

Look into the Chmod function, it's because bits from fs.FileMode can't be used as syscall file mode bits directly, it will cause some bits missing.

@cyphar
Copy link
Member

cyphar commented Aug 3, 2023

Not yet backported, reopening.

@kolyshkin
Copy link
Contributor

Fixed by #3961. Will be part of 1.1.9.

@ohookins
Copy link
Author

ohookins commented Aug 3, 2023

Thanks everyone!

kolyshkin referenced this issue Aug 10, 2023
When a directory already exists (or after a container is restarted) the
perms of the directory being mounted to were being used even when a
different permission is set on the tmpfs mount options.

This prepends the original directory perms to the mount options.
If the perms were already set in the mount opts then those perms will
win.
This eliminates the need to perform a chmod after mount entirely.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants