From d7846bc19a51b21b46803b4fd6a68b7ca31c33af Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 16 Nov 2021 13:50:09 -0800 Subject: [PATCH] Fix failure with rw bind mount of a ro fuse As reported in [1], in a case where read-only fuse (sshfs) mount is used as a volume without specifying ro flag, the kernel fails to remount it (when adding various flags such as nosuid and nodev), returning EPERM. Here's the relevant strace line: > [pid 333966] mount("/tmp/bats-run-PRVfWc/runc.RbNv8g/bundle/mnt", "/proc/self/fd/7", 0xc0001e9164, MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC, NULL) = -1 EPERM (Operation not permitted) I was not able to reproduce it with other read-only mounts as the source (tried tmpfs, read-only bind mount, and an ext2 mount), so somehow this might be specific to fuse. The fix is to check whether the source has RDONLY flag, and retry the remount with this flag added. A test case (which was kind of hard to write) is added, and it fails without the fix. Note that rootless user need to be able to ssh to rootless@localhost in order to sshfs to work -- amend setup scripts to make it work. [1] https://github.com/containers/podman/issues/12205 Signed-off-by: Kir Kolyshkin --- .cirrus.yml | 8 +++++++- .github/workflows/test.yml | 7 ++++--- Dockerfile | 1 + Vagrantfile.fedora | 5 +++-- libcontainer/rootfs_linux.go | 17 ++++++++++++++++- tests/integration/mounts.bats | 30 ++++++++++++++++++++++++++++++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index bf9975a4f74..77ce98d21b8 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -99,7 +99,7 @@ task: # Work around dnf mirror failures by retrying a few times. for i in $(seq 0 2); do sleep $i - yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make criu && break + yum install -y -q gcc git iptables jq glibc-static libseccomp-devel make criu fuse-sshfs && break done [ $? -eq 0 ] # fail if yum failed # install Go @@ -113,6 +113,12 @@ task: cd - # Add a user for rootless tests useradd -u2000 -m -d/home/rootless -s/bin/bash rootless + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh + ssh-keygen -t ecdsa -N "" -f /root/rootless.key + mkdir -m 0700 -p /home/rootless/.ssh + cp /root/rootless.key /home/rootless/.ssh/id_ecdsa + cat /root/rootless.key.pub >> /home/rootless/.ssh/authorized_keys + chown -R rootless.rootless /home/rootless # set PATH echo 'export PATH=/usr/local/go/bin:/usr/local/bin:$PATH' >> /root/.bashrc # Setup ssh localhost for terminal emulation (script -e did not work) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c5c29356c34..72f6eb377a9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -46,13 +46,13 @@ jobs: curl -fSsl $REPO/Release.key | sudo apt-key add - echo "deb $REPO/ /" | sudo tee /etc/apt/sources.list.d/criu.list sudo apt update - sudo apt install libseccomp-dev criu + sudo apt install libseccomp-dev criu sshfs - name: install deps (criu ${{ matrix.criu }}) if: matrix.criu != '' run: | sudo apt -q update - sudo apt -q install libseccomp-dev \ + sudo apt -q install libseccomp-dev sshfs \ libcap-dev libnet1-dev libnl-3-dev \ libprotobuf-c-dev libprotobuf-dev protobuf-c-compiler protobuf-compiler git clone https://github.com/checkpoint-restore/criu.git ~/criu @@ -81,9 +81,10 @@ jobs: if: matrix.rootless == 'rootless' run: | sudo useradd -u2000 -m -d/home/rootless -s/bin/bash rootless - # Allow root to execute `ssh rootless@localhost` in tests/rootless.sh + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh ssh-keygen -t ecdsa -N "" -f $HOME/rootless.key sudo mkdir -m 0700 -p /home/rootless/.ssh + sudo cp $HOME/rootless.key /home/rootless/.ssh/id_ecdsa sudo cp $HOME/rootless.key.pub /home/rootless/.ssh/authorized_keys sudo chown -R rootless.rootless /home/rootless diff --git a/Dockerfile b/Dockerfile index 03475b8b3bd..2ba1ba12874 100644 --- a/Dockerfile +++ b/Dockerfile @@ -31,6 +31,7 @@ RUN KEYFILE=/usr/share/keyrings/criu-repo-keyring.gpg; \ kmod \ pkg-config \ python3-minimal \ + sshfs \ sudo \ uidmap \ && apt-get clean \ diff --git a/Vagrantfile.fedora b/Vagrantfile.fedora index 2c3df9abc19..69d36e672ad 100644 --- a/Vagrantfile.fedora +++ b/Vagrantfile.fedora @@ -27,7 +27,7 @@ Vagrant.configure("2") do |config| cat << EOF | dnf -y --exclude=kernel,kernel-core,systemd,systemd-* shell && break config install_weak_deps false update -install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu +install iptables gcc make golang-go glibc-static libseccomp-devel bats jq git-core criu fuse-sshfs ts run EOF done @@ -36,9 +36,10 @@ EOF # Add a user for rootless tests useradd -u2000 -m -d/home/rootless -s/bin/bash rootless - # Allow root to execute `ssh rootless@localhost` in tests/rootless.sh + # Allow root and rootless itself to execute `ssh rootless@localhost` in tests/rootless.sh ssh-keygen -t ecdsa -N "" -f /root/rootless.key mkdir -m 0700 -p /home/rootless/.ssh + cp /root/rootless.key /home/rootless/.ssh/id_ecdsa cat /root/rootless.key.pub >> /home/rootless/.ssh/authorized_keys chown -R rootless.rootless /home/rootless diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 63322d01da3..2832b8291a4 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1065,7 +1065,22 @@ func remount(m *configs.Mount, rootfs string, mountFd *int) error { } return utils.WithProcfd(rootfs, m.Destination, func(procfd string) error { - return mount(source, m.Destination, procfd, m.Device, uintptr(m.Flags|unix.MS_REMOUNT), "") + flags := uintptr(m.Flags | unix.MS_REMOUNT) + err := mount(source, m.Destination, procfd, m.Device, flags, "") + if err == nil { + return nil + } + // Check if the source has ro flag... + var s unix.Statfs_t + if err := unix.Statfs(source, &s); err != nil { + return &os.PathError{Op: "statfs", Path: source, Err: err} + } + if s.Flags&unix.MS_RDONLY != unix.MS_RDONLY { + return err + } + // ... and retry the mount with ro flag set. + flags |= unix.MS_RDONLY + return mount(source, m.Destination, procfd, m.Device, flags, "") }) } diff --git a/tests/integration/mounts.bats b/tests/integration/mounts.bats index f21136b6042..5f04843c8ca 100644 --- a/tests/integration/mounts.bats +++ b/tests/integration/mounts.bats @@ -7,6 +7,11 @@ function setup() { } function teardown() { + if [ -d mnt ]; then + # New distros (Fedora 35) do not have fusermount installed + # as a dependency of fuse-sshfs, and good ol' umount works. + fusermount -u mnt || umount mnt + fi teardown_bundle } @@ -52,3 +57,28 @@ function teardown() { runc run test_busybox [ "$status" -eq 0 ] } + +@test "runc run [rw bind mount of a ro fuse sshfs mount]" { + requires rootless + + DIR="$PWD/mnt" + mkdir "$DIR" + # Create a ro fuse-sshfs mount. + sshfs \ + -o UserKnownHostsFile=/dev/null \ + -o StrictHostKeyChecking=no \ + -o PasswordAuthentication=no \ + -o ro \ + rootless@localhost: "$DIR" + + update_config ' .mounts += [{ + type: "bind", + source: "'"$DIR"'", + destination: "/mnt", + options: ["rw", "rprivate", "nosuid", "nodev", "rbind"] + }] + | .process.args |= ["true"]' + + runc run test_busybox + [ "$status" -eq 0 ] +}