Fix issues with mountpoint handling

[dpward]

These fixes ensure that essential mountpoints are unmounted, in the event an exception is raised while one of them is being mounted. They also work around a known issue with util-linux versions before 2.33 (in RHEL 7 and 8) that affects recursive bind-mounting when the propagation is specified.

[praiskup]

Can you please describe how to reproduce the issue you are fixing with the second patch?

[praiskup]

I mean, I definitely tested the code on EL7 and EL8. It would be nice to have a reproducer in the testsuite.

[praiskup]

The first patch looks good, btw., thank you for the PR!

[praiskup]

FTR, there's also 34fe0d5. I can see that split into two separate mount options was later suggested in rhbz#1815534...

[dpward]

On a clean installation of CentOS 8.3, these are the mounts in the host:

$ mount
sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,relatime,seclabel)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
devtmpfs on /dev type devtmpfs (rw,nosuid,seclabel,size=3886868k,nr_inodes=971717,mode=755)
securityfs on /sys/kernel/security type securityfs (rw,nosuid,nodev,noexec,relatime)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev,seclabel)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,seclabel,gid=5,mode=620,ptmxmode=000)
tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,seclabel,mode=755)
cgroup on /sys/fs/cgroup/systemd type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd)
pstore on /sys/fs/pstore type pstore (rw,nosuid,nodev,noexec,relatime,seclabel)
efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)
bpf on /sys/fs/bpf type bpf (rw,nosuid,nodev,noexec,relatime,mode=700)
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,cpu,cpuacct)
cgroup on /sys/fs/cgroup/perf_event type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,perf_event)
cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,blkio)
cgroup on /sys/fs/cgroup/net_cls,net_prio type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,net_cls,net_prio)
cgroup on /sys/fs/cgroup/freezer type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,freezer)
cgroup on /sys/fs/cgroup/rdma type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,rdma)
cgroup on /sys/fs/cgroup/pids type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,pids)
cgroup on /sys/fs/cgroup/cpuset type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,cpuset)
cgroup on /sys/fs/cgroup/hugetlb type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,hugetlb)
cgroup on /sys/fs/cgroup/memory type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,memory)
cgroup on /sys/fs/cgroup/devices type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,devices)
none on /sys/kernel/tracing type tracefs (rw,relatime,seclabel)
configfs on /sys/kernel/config type configfs (rw,relatime)
/dev/mapper/cl_localhost-root on / type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
selinuxfs on /sys/fs/selinux type selinuxfs (rw,relatime)
systemd-1 on /proc/sys/fs/binfmt_misc type autofs (rw,relatime,fd=27,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=21101)
debugfs on /sys/kernel/debug type debugfs (rw,relatime,seclabel)
hugetlbfs on /dev/hugepages type hugetlbfs (rw,relatime,seclabel,pagesize=2M)
mqueue on /dev/mqueue type mqueue (rw,relatime,seclabel)
fusectl on /sys/fs/fuse/connections type fusectl (rw,relatime)
/dev/sda9 on /boot type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/sda1 on /boot/efi type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=ascii,shortname=winnt,errors=remount-ro)
sunrpc on /var/lib/nfs/rpc_pipefs type rpc_pipefs (rw,relatime)
tmpfs on /run/user/42 type tmpfs (rw,nosuid,nodev,relatime,seclabel,size=783712k,mode=700,uid=42,gid=42)
tmpfs on /run/user/1000 type tmpfs (rw,nosuid,nodev,relatime,seclabel,size=783712k,mode=700,uid=1000,gid=1000)
gvfsd-fuse on /run/user/1000/gvfs type fuse.gvfsd-fuse (rw,nosuid,nodev,relatime,user_id=1000,group_id=1000)

Notice all of the submounts that are underneath /sys and /proc. But mock fails to perform a recursive bind-mount:

$ mock --isolation=simple --no-bootstrap-chroot --chroot mount
INFO: mock.py version 2.9 starting (python version = 3.6.8, NVR = mock-2.9-1.el8)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
INFO: Signal handler active	
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled package manager cache
Start: cleaning package manager metadata
Finish: cleaning package manager metadata
INFO: enabled HW Info plugin
Mock Version: 2.9
INFO: Mock Version: 2.9
Finish: chroot init
INFO: Running in chroot: ['mount']
Start: chroot ['mount']
tmpfs on /proc type tmpfs (rw,relatime,seclabel)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
tmpfs on /sys type tmpfs (rw,relatime,seclabel)
sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,relatime,seclabel)
tmpfs on /dev/shm type tmpfs (rw,relatime,seclabel)
devpts on /dev/pts type devpts (rw,relatime,seclabel,gid=5,mode=620,ptmxmode=666)
tmpfs on /sys/fs/selinux type tmpfs (rw,relatime,seclabel)
/dev/mapper/cl_localhost-root on /var/cache/yum type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/mapper/cl_localhost-root on /var/cache/dnf type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/mapper/cl_localhost-root on /proc/filesystems type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
Finish: chroot ['mount']

Now apply the second patch in this PR:

$ curl -s https://github.com/rpm-software-management/mock/commit/ac1adfa5fd73.patch | sudo patch -p4 -d /usr/lib/python3.6/site-packages/mockbuild/
patching file mounts.py

and then mock will do the recursive bind-mount:

$ mock --isolation=simple --no-bootstrap-chroot --chroot mount
INFO: mock.py version 2.9 starting (python version = 3.6.8, NVR = mock-2.9-1.el8)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
INFO: Signal handler active
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled package manager cache
Start: cleaning package manager metadata
Finish: cleaning package manager metadata
INFO: enabled HW Info plugin
Mock Version: 2.9
INFO: Mock Version: 2.9
Finish: chroot init
INFO: Running in chroot: ['mount']
Start: chroot ['mount']
tmpfs on /proc type tmpfs (rw,relatime,seclabel)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
systemd-1 on /proc/sys/fs/binfmt_misc type autofs (rw,relatime,fd=27,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=21101)
tmpfs on /sys type tmpfs (rw,relatime,seclabel)
sysfs on /sys type sysfs (rw,nosuid,nodev,noexec,relatime,seclabel)
securityfs on /sys/kernel/security type securityfs (rw,nosuid,nodev,noexec,relatime)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,seclabel,mode=755)
cgroup on /sys/fs/cgroup/systemd type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd)
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,cpu,cpuacct)
cgroup on /sys/fs/cgroup/perf_event type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,perf_event)
cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,blkio)
cgroup on /sys/fs/cgroup/net_cls,net_prio type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,net_cls,net_prio)
cgroup on /sys/fs/cgroup/freezer type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,freezer)
cgroup on /sys/fs/cgroup/rdma type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,rdma)
cgroup on /sys/fs/cgroup/pids type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,pids)
cgroup on /sys/fs/cgroup/cpuset type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,cpuset)
cgroup on /sys/fs/cgroup/hugetlb type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,hugetlb)
cgroup on /sys/fs/cgroup/memory type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,memory)
cgroup on /sys/fs/cgroup/devices type cgroup (rw,nosuid,nodev,noexec,relatime,seclabel,devices)
pstore on /sys/fs/pstore type pstore (rw,nosuid,nodev,noexec,relatime,seclabel)
efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)
bpf on /sys/fs/bpf type bpf (rw,nosuid,nodev,noexec,relatime,mode=700)
none on /sys/kernel/tracing type tracefs (rw,relatime,seclabel)
configfs on /sys/kernel/config type configfs (rw,relatime)
selinuxfs on /sys/fs/selinux type selinuxfs (rw,relatime)
debugfs on /sys/kernel/debug type debugfs (rw,relatime,seclabel)
fusectl on /sys/fs/fuse/connections type fusectl (rw,relatime)
tmpfs on /dev/shm type tmpfs (rw,relatime,seclabel)
devpts on /dev/pts type devpts (rw,relatime,seclabel,gid=5,mode=620,ptmxmode=666)
tmpfs on /sys/fs/selinux type tmpfs (rw,relatime,seclabel)
/dev/mapper/cl_localhost-root on /var/cache/yum type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/mapper/cl_localhost-root on /var/cache/dnf type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/mapper/cl_localhost-root on /proc/filesystems type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
Finish: chroot ['mount']

[praiskup]

Thanks, is this causing any real issues? Build failures?

[dpward]

Thanks, is this causing any real issues? Build failures?

If mock does not undo its mounts correctly, then they are left behind when it exits. At that point, it is not possible to run mock again on the same chroot without manually removing those mounts (as the root user).

As far as recursive bind mounting, I noticed this while examining the code, not directly because of something I encountered during a build. But, the function does not work as expected (it's a bug); it will produce different behavior when the host is Fedora vs. RHEL, and we do have an easy well-known fix here (which might be considered better: one system call per command). Since this is a generic function, we may use it in the future to perform recursive bind-mounts aside from /proc and /sys. It would be nice to expose the recursive parameter to users through the bind_mount plugin.

[praiskup]

the function does not work as expected (it's a bug)

Yeah, I agree - I'm asking for a reproducer so I can create a new test-case. But I think it is fair if we go without the test here...

[praiskup]

Thank you!

[praiskup]

Ok, reading the manual page once more, I'm afraid the second patch was wrong - at least from recursive bind-mount perspective. Per mount(8) manual page:

   It's impossible to change mount options recursively (for  example  with
       -o rbind,ro).

From just bind (not rbind) perspective - indeed, we need to first bind and then set the options with remount.
@dpward do you agree?

[dpward]

Ok, reading the manual page once more, I'm afraid the second patch was wrong - at least from recursive bind-mount perspective. Per mount(8) manual page:

   It's impossible to change mount options recursively (for  example  with
       -o rbind,ro).

From just bind (not rbind) perspective - indeed, we need to first bind and then set the options with remount.
@dpward do you agree?

It's saying that mount options can't be applied recursively at all — even with mount --rbind -o ro. This is a limitation of the underlying system call. Commit 163b24e shouldn't make a difference here.

This is a separate problem that I had described in #713.

[praiskup]

I mean - I am reading the PR commit over again, and it talks about recursive bind-mounts;
and about configuring propagation. Since it has split the single mount command into mount + remount
-- the remount part together with rbind will have no effect for mount options configs, or what am I missing?

[dpward]

Mount propagation options (such as rshared or rprivate) can be set recursively. However, mount options (such as ro or nosuid) cannot.

In a working version of util-linux, this command:

mount --rbind -o rprivate,ro /src /dest

does exactly the same thing as:

mount -o rbind /src /dest
mount -o remount,rprivate,ro /src /dest

Either method will cause the same two system calls to be issued. It will make /dest and all of its submounts private. However, it will only apply the ro option to /dest, not to any of its submounts.

The problem here was that there were non-working versions of util-linux, which did not correctly parse --rbind and -o rprivate in the same mount command. To work around this, the second syntax above is used instead of the first.