Skip to content

Don't write bind mounts to /etc/fstab for nodev - #15322

Open
Chr0mium24 wants to merge 1 commit into
ComplianceAsCode:masterfrom
Chr0mium24:fix-nodev-nonroot-bind-mounts
Open

Chr0mium24 wants to merge 1 commit into
ComplianceAsCode:masterfrom
Chr0mium24:fix-nodev-nonroot-bind-mounts

Conversation

@Chr0mium24

Copy link
Copy Markdown

Description:

  • In the Bash remediation of mount_option_nodev_nonroot_local_partitions, skip a mount whose findmnt source has the form DEVICE[PATH] (a bind mount, or a mount of a subdirectory or a btrfs subvolume) if its mount point has no /etc/fstab entry. If it has one, the remediation adds nodev to that entry, the same as before.
  • Add two test scenarios: a bind mount with no /etc/fstab entry, and a bind mount that has its own /etc/fstab entry without nodev.

Rationale:

  • The remediation gets the mounts from findmnt --mtab --raw --evaluate and keeps the records whose source matches \s/dev/\w. When the root of a mount inside its file system is not /, findmnt prints the source as SPEC[ROOT], for example /dev/mapper/vg0-root[/srv/b]. If the mount point is not in /etc/fstab, bash_ensure_mount_option_in_fstab appends $device $mount_point $device_type defaults,...,nodev 0 0, and the [...] source ends up in /etc/fstab. On the next boot systemd waits 90 s for a device that doesn't exist, local-fs.target fails, and the host drops into emergency mode. Podman hosts hit this because containers/storage bind-mounts its overlay directory onto itself.

  • A mount like that can't be recreated from its findmnt source, so the remediation now leaves it alone unless /etc/fstab already has an entry for the mount point. The new check uses the same regular expression that bash_ensure_mount_option_in_fstab uses to choose between "append a new entry" and "edit the existing entry", so the append branch can't be reached with a [PATH] source. Mounts that have an fstab entry, such as btrfs subvolumes mounted with subvol= and bind mounts listed in fstab, go through the same code as before.

  • findmnt has no option to leave out bind mounts. --nofsroot only removes the [PATH] suffix, and that would turn the record into /dev/mapper/vg0-root /srv/b xfs ..., an fstab entry that mounts the whole root file system on /srv/b. The suffix is the only marker findmnt gives. In misc-utils/findmnt.c, the SOURCE column is printed as %s[%s] whenever the fs root is not / and --nofsroot isn't set. I checked the source of util-linux v2.23 (RHEL 7 / OL 7), v2.32 (RHEL 8), v2.40 (RHEL 10) and current master, and they all do this. The fix adds no findmnt option or new tool, only a bash pattern match and a grep -q like the ones the script already uses. The rendered script is byte-identical for rhel8, rhel9, rhel10, ol7, ol8, almalinux9, fedora, sle12, sle15, sle16, ubuntu2204, ubuntu2404, debian11 and debian12.

  • Not changed here: the OVAL check reads /proc/mounts, where a bind mount shows the underlying device. So a bind mount that exists only at runtime and has no nodev (for example the Podman one, when its source file system is mounted without nodev) is still reported after remediation. Before this change it was reported as fixed, but the fstab entry left behind made the host unbootable.

  • Fixes mount_option_nodev_nonroot_local_partitions writes bind mounts into /etc/fstab (DEVICE[PATH] source), host boots into emergency mode #15292

Review Hints:

  • Automatus was not run, because I had no VM. The new scenarios are untested. Here is what they should show:
    • bind_mount_not_in_fstab.fail.sh should pass on master too. The fstab part of the check accepts any /dev/\S+ source, so it can't see the bad line. The scenario covers remediation with such a bind mount present and keeps that case in the test set.
    • bind_mount_in_fstab_without_nodev.fail.sh covers the "mount point already in fstab" path. It should pass with and without this change, and fail if [PATH] sources were skipped unconditionally.
  • I reproduced the bug and checked the fix with a sandbox harness. The remediation was rendered with ssg.jinja and the rhel9 product environment, the same way the build renders it but without the container/bootc platform wrapper. It then ran as a non-root user on Ubuntu 22.04 (GNU bash 5.1.16, GNU sed and grep, util-linux 2.37.2) against temporary copies of /etc/fstab, /etc/mtab and /etc/security/namespace.conf, with findmnt replaced by canned output and mount, mountpoint and mkdir replaced by logging stubs. On the same host, the real findmnt --mtab --raw --evaluate prints sources such as tmpfs[/snapd/ns], so the canned DEVICE[PATH] records match what util-linux prints.
    • The issue's reproducer (/ on /dev/mapper/vg0-root, plus mount --bind /srv/b /srv/b): on master the remediation appends /dev/mapper/vg0-root[/srv/b] /srv/b xfs defaults,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,nodev 0 0, the same line as in the issue. With this change, /etc/fstab is unchanged.
    • A mixed host: /home missing nodev, /var/log with nodev, a btrfs subvolume /opt (/dev/vdc1[/@opt], UUID= entry in fstab), a bind mount /srv/www listed in fstab, /data mounted but not in fstab, gfs2, nfs4, /boot, /boot/efi, a polyinstantiated /tmp/tmp-inst, /srv/b and /var/lib/containers/storage/overlay. Master appends the /srv/b and overlay lines with [PATH] sources. With this change those two are skipped and nothing else changes: /home, /opt and /srv/www get nodev in their existing entries, and /data is appended as /dev/vdb1 /data ext4 defaults,relatime,nodev 0 0.
    • A bind mount whose only fstab line is commented out: master appends the [PATH] line. With this change, nothing is written.
  • shellcheck -s bash -S warning --extended-analysis=false (the options of the <product>-bash-shellcheck ctest) reports nothing on the rendered script before or after the change. At default severity, the findings are the same before and after. tests/ensure_bash_remediations_do_not_exit.py, tests/ensure_paths_are_short.py, utils/check_eof.py and utils/check_smart_quotes.py pass.
  • The Ansible remediation is not changed. From reading Ansible's mount facts code, a bind mount is reported with the underlying device and ,bind added to its options, so ansible.posix.mount may write a different broken entry. I haven't verified this. If it turns out to be true, it can be handled separately.
  • To test: ./build_product rhel9 --datastream-only, then ./tests/automatus.py rule --libvirt qemu:///system <vm> --datastream build/ssg-rhel9-ds.xml mount_option_nodev_nonroot_local_partitions.

The Bash remediation of mount_option_nodev_nonroot_local_partitions
takes the source of every local mount reported by findmnt and, if the
mount point has no /etc/fstab entry, appends a new entry with that
source. For bind mounts and other mounts of a file system subtree,
findmnt reports the source as DEVICE[PATH], for example
/dev/mapper/vg0-root[/srv/b]. Such an entry names a device that does
not exist, so on the next boot systemd waits for it, local-fs.target
fails and the system ends up in emergency mode.

Skip mounts whose source has the [PATH] suffix unless the mount point
already has an /etc/fstab entry. In that case the remediation only
adds nodev to the existing entry, as before, so btrfs subvolumes and
bind mounts that are listed in /etc/fstab are still remediated.

Resolves: ComplianceAsCode#15292
@openshift-ci openshift-ci Bot added the needs-ok-to-test Used by openshift-ci bot. label Nov 9, 2026
@openshift-ci

openshift-ci Bot commented Nov 9, 2026

Copy link
Copy Markdown

Hi @Chr0mium24. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@github-actions

Copy link
Copy Markdown
Contributor

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_mount_option_nodev_nonroot_local_partitions' differs.
--- xccdf_org.ssgproject.content_rule_mount_option_nodev_nonroot_local_partitions
+++ xccdf_org.ssgproject.content_rule_mount_option_nodev_nonroot_local_partitions
@@ -57,6 +57,17 @@
         fi
     done
 
+    # findmnt shows bind mounts and other mounts of a file system subtree,
+    # such as btrfs subvolumes, as DEVICE[PATH]. That is not a device which
+    # can be written to /etc/fstab, so such a mount is only remediated when
+    # it already has an entry in /etc/fstab.
+    if [[ "$device" == *\[*\] ]]; then
+        fstab_entry_regexp="$(printf "^[[:space:]]*[^#].*[[:space:]]%s[[:space:]]" "$mount_point")"
+        if ! grep -q "$fstab_entry_regexp" /etc/fstab; then
+            continue
+        fi
+    fi
+
     # If we reach here, it's a local, non-root partition that isn't excluded.
     mount_point_match_regexp="$(printf "^[[:space:]]*[^#].*[[:space:]]%s[[:space:]]" $mount_point)"
 

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Used by openshift-ci bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mount_option_nodev_nonroot_local_partitions writes bind mounts into /etc/fstab (DEVICE[PATH] source), host boots into emergency mode

1 participant