Skip to content

Commit

Permalink
Fix failure with rw bind mount of a ro fuse
Browse files Browse the repository at this point in the history
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, and skip the test if the setup is not working.

[1] containers/podman#12205

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Nov 18, 2021
1 parent 20e9288 commit 50105de
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 7 deletions.
8 changes: 7 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ RUN KEYFILE=/usr/share/keyrings/criu-repo-keyring.gpg; \
kmod \
pkg-config \
python3-minimal \
sshfs \
sudo \
uidmap \
&& apt-get clean \
Expand Down
5 changes: 3 additions & 2 deletions Vagrantfile.fedora
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
17 changes: 16 additions & 1 deletion libcontainer/rootfs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
})
}

Expand Down
40 changes: 40 additions & 0 deletions tests/integration/mounts_sshfs.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env bats

load helpers

function setup() {
# Create a ro fuse-sshfs mount; skip the test if it's not working.
local sshfs="sshfs
-o UserKnownHostsFile=/dev/null
-o StrictHostKeyChecking=no
-o PasswordAuthentication=no"

DIR="$BATS_RUN_TMPDIR/fuse-sshfs"
mkdir -p "$DIR"

if ! $sshfs -o ro rootless@localhost: "$DIR"; then
skip "test requires working sshfs mounts"
fi

setup_hello
}

function teardown() {
# New distros (Fedora 35) do not have fusermount installed
# as a dependency of fuse-sshfs, and good ol' umount works.
fusermount -u "$DIR" || umount "$DIR"

teardown_bundle
}

@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
update_config ' .mounts += [{
type: "bind",
source: "'"$DIR"'",
destination: "/mnt",
options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
}]'

runc run test_busybox
[ "$status" -eq 0 ]
}

0 comments on commit 50105de

Please sign in to comment.