This code appears in clean_run: mount --bind / $dir && rm -rf $dir/run/* || rc=1 umount $dir rm -rf $dir Should the umount possibly fail somehow (not sure how or even if that’s possible given that it’s a bind mount—but maybe if the umount binary is corrupt or missing?), its return value isn’t checked. Why not use rmdir instead? $dir is made with mktemp -d and therefore is guaranteed to start out empty. If the umount does somehow fail, leaving a bind-mount lying around is a whole lot friendlier than wiping your hard drive. Reproducible: Always $ emerge --info sys-apps/openrc Portage 2.2.20.1 (python 3.4.1-final-0, default/linux/amd64/13.0, gcc-4.8.5, glibc-2.20-r2, 4.0.5-gentoo x86_64) ================================================================= System Settings ================================================================= System uname: Linux-4.0.5-gentoo-x86_64-Intel-R-_Core-TM-_i7_CPU_Q_720_@_1.60GHz-with-gentoo-2.2 KiB Mem: 6034780 total, 4045472 free KiB Swap: 8390452 total, 8390452 free Timestamp of repository gentoo: Sun, 06 Sep 2015 04:15:01 +0000 sh bash 4.3_p39 ld GNU ld (Gentoo 2.24 p1.4) 2.24 app-shells/bash: 4.3_p39::gentoo dev-java/java-config: 2.2.0::gentoo dev-lang/perl: 5.20.2::gentoo dev-lang/python: 2.7.9-r1::gentoo, 3.4.1::gentoo dev-util/cmake: 3.2.2::gentoo dev-util/pkgconfig: 0.28-r2::gentoo sys-apps/baselayout: 2.2::gentoo sys-apps/openrc: 0.17::gentoo sys-apps/sandbox: 2.6-r1::gentoo sys-devel/autoconf: 2.13::gentoo, 2.69::gentoo sys-devel/automake: 1.11.6-r1::gentoo, 1.14.1::gentoo, 1.15::gentoo sys-devel/binutils: 2.24-r3::gentoo sys-devel/gcc: 4.8.5::gentoo sys-devel/gcc-config: 1.7.3::gentoo sys-devel/libtool: 2.4.6::gentoo sys-devel/make: 4.1-r1::gentoo sys-kernel/linux-headers: 3.18::gentoo (virtual/os-headers) sys-libs/glibc: 2.20-r2::gentoo Repositories: steam-overlay location: /var/cache/repos/steam sync-type: git sync-uri: https://github.com/anyc/steam-overlay.git masters: gentoo priority: 1 sunrise location: /var/cache/repos/sunrise sync-type: git sync-uri: https://anongit.gentoo.org/git/proj/sunrise-reviewed.git masters: gentoo priority: 2 gentoo location: /var/cache/repos/gentoo sync-type: rsync sync-uri: rsync://rsync.ca.gentoo.org/gentoo-portage priority: 3 hawk777 location: /var/cache/repos/hawk777 sync-type: svn sync-uri: svn://chead.ca/portage/trunk masters: gentoo priority: 4 kruskal-local location: /usr/local/portage masters: gentoo priority: 5 Installed sets: @admin, @android, @arm, @cups, @desktop, @games, @linuxcnc, @lisp, @magicalgame, @media, @misc, @net, @office, @web ACCEPT_KEYWORDS="amd64" ACCEPT_LICENSE="* -@EULA AdobeFlash-11.x dlj-1.1 Google-TOS Oracle-BCLA-JavaSE" CBUILD="x86_64-pc-linux-gnu" CFLAGS="-O2 -pipe -march=native -fomit-frame-pointer" CHOST="x86_64-pc-linux-gnu" CONFIG_PROTECT="/etc /usr/lib64/libreoffice/program/sofficerc /usr/share/gnupg/qualified.txt /var/lib/hsqldb" CONFIG_PROTECT_MASK="/etc/ca-certificates.conf /etc/dconf /etc/env.d /etc/fonts/fonts.conf /etc/gconf /etc/gentoo-release /etc/revdep-rebuild /etc/sandbox.d /etc/terminfo /etc/texmf/language.dat.d /etc/texmf/language.def.d /etc/texmf/updmap.d /etc/texmf/web2c" CXXFLAGS="-O2 -pipe -march=native -fomit-frame-pointer" DISTDIR="/var/cache/distfiles" EMERGE_DEFAULT_OPTS="--jobs 2 --load-average 4 --with-bdeps y --complete-graph y -1" FCFLAGS="-O2 -pipe" FEATURES="assume-digests binpkg-logs config-protect-if-modified distlocks ebuild-locks fakeroot fixlafiles merge-sync news parallel-fetch preserve-libs protect-owned sandbox sfperms sign strict unknown-features-warn unmerge-logs unmerge-orphans userfetch userpriv usersandbox usersync xattr" FFLAGS="-O2 -pipe" GENTOO_MIRRORS="ftp://gentoo.arcticnetwork.ca/pub/gentoo ftp://mirror.csclub.uwaterloo.ca/gentoo-distfiles http://gentoo.osuosl.org/" INSTALL_MASK="grub2-mkconfig" LANG="en_CA.UTF-8" LDFLAGS="-Wl,-O1 -Wl,--as-needed" MAKEOPTS="--jobs 2 --load-average 4" PKGDIR="/usr/portage/packages" PORTAGE_CONFIGROOT="/" PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --omit-dir-times --compress --force --whole-file --delete --stats --human-readable --timeout=180 --exclude=/distfiles --exclude=/local --exclude=/packages" PORTAGE_TMPDIR="/var/tmp" USE="X a52 aac acl acpi aio alsa amd64 archive bluetooth bzip2 cairo caps cdda cddb cdinstall cdio cli css cups cxx dbus double-precision dri dvd faac faad fat ffmpeg fftw flac fontconfig fortran gcrypt gif gimp glade gmp gnutls gtk hou iconv icu idn imap ipv6 java joystick jpeg laptop libnotify libsamplerate lm_sensors mad mbox mikmod mmx mmxext modules mp3 mpeg multilib multiuser ncurses nls nntp nowin nptl ntfs ntp offensive ogg opengl openmp pam pdf plotutils png qt3support readline scanner sdl seccomp session sou spell sse sse2 ssl startup-notification stream svg symlink taglib theora threads thunar tiff truetype udev unicode usb vdpau videos vim-syntax vorbis win32codecs xattr xetex xft xinerama xscreensaver xulrunner xv xvid xvmc zlib" ABI_X86="64" ALSA_CARDS="ali5451 als4000 atiixp atiixp-modem bt87x ca0106 cmipci emu10k1x ens1370 ens1371 es1938 es1968 fm801 hda-intel intel8x0 intel8x0m maestro3 trident usb-audio via82xx via82xx-modem ymfpci" APACHE2_MODULES="authn_core authz_core socache_shmcb unixd actions alias auth_basic authn_alias authn_anon authn_dbm authn_default authn_file authz_dbm authz_default authz_groupfile authz_host authz_owner authz_user autoindex cache cgi cgid dav dav_fs dav_lock deflate dir disk_cache env expires ext_filter file_cache filter headers include info log_config logio mem_cache mime mime_magic negotiation rewrite setenvif speling status unique_id userdir usertrack vhost_alias" CALLIGRA_FEATURES="kexi words flow plan sheets stage tables krita karbon braindump author" CAMERAS="ptp2" COLLECTD_PLUGINS="df interface irq load memory rrdtool swap syslog" CPU_FLAGS_X86="mmx mmxext popcnt sse sse2 sse3 sse4_1 sse4_2 ssse3" ELIBC="glibc" GPSD_PROTOCOLS="ashtech aivdm earthmate evermore fv18 garmin garmintxt gpsclock itrax mtk3301 nmea ntrip navcom oceanserver oldstyle oncore rtcm104v2 rtcm104v3 sirf superstar2 timing tsip tripmate tnt ublox ubx" GRUB_PLATFORMS="pc" INPUT_DEVICES="evdev synaptics" KERNEL="linux" LCD_DEVICES="bayrad cfontz cfontz633 glk hd44780 lb216 lcdm001 mtxorb ncurses text" LIBREOFFICE_EXTENSIONS="presenter-console presenter-minimizer" LINGUAS="en en_CA" OFFICE_IMPLEMENTATION="libreoffice" PHP_TARGETS="php5-5" PYTHON_SINGLE_TARGET="python2_7" PYTHON_TARGETS="python2_7 python3_4" QEMU_SOFTMMU_TARGETS="i386 x86_64" RUBY_TARGETS="ruby19 ruby20" SANE_BACKENDS="plustek" USERLAND="GNU" VIDEO_CARDS="nvidia" XFCE_PLUGINS="logout menu" XTABLES_ADDONS="quota2 psd pknock lscan length2 ipv4options ipset ipp2p iface geoip fuzzy condition tee tarpit sysrq steal rawnat logmark ipmark dhcpmac delude chaos account" Unset: CC, CPPFLAGS, CTARGET, CXX, LC_ALL, PORTAGE_BUNZIP2_COMMAND, PORTAGE_COMPRESS, PORTAGE_COMPRESS_FLAGS, PORTAGE_RSYNC_EXTRA_OPTS, USE_PYTHON ================================================================= Package Settings ================================================================= sys-apps/openrc-0.17::gentoo was built with the following: USE="ncurses pam unicode -audit -debug -netifrc -newnet (-prefix) (-selinux) -static-libs -tools" ABI_X86="64"
Why do maintainer using > rm -rf $dir instead of > rmdir $dir # assuming $dir is empty, or it anyway shouldn't delete it otherwise ? And yes, it should also use "&&" after umount // although, calling `rm` on *anything* on the live system during installation is unacceptable for me. There is postinst for such things.
(In reply to Vadim A. Misbakh-Soloviov (mva) from comment #1) > // although, calling `rm` on *anything* on the live system during > installation is unacceptable for me. There is postinst for such things. This isn’t during installation. This is at every boot.
(In reply to Christopher Head from comment #2) > This isn’t during installation. This is at every boot. So, then removing is kinda acceptable, but, anyway, IMHO, it should use: `mountpoint -q $dev || rm` but not just `rm` (and, IMHO, -f switch is unneeded in such scripts). // I'll try to ping oRC devs in IRC about the bug
(In reply to Vadim A. Misbakh-Soloviov (mva) from comment #3) > (In reply to Christopher Head from comment #2) > > This isn’t during installation. This is at every boot. > > So, then removing is kinda acceptable, but, anyway, IMHO, it should use: > `mountpoint -q $dev || rm` but not just `rm` (and, IMHO, -f switch is > unneeded in such scripts). Better would be rmdir; rm doesn’t remove directories at all (even if empty) unless -r is provided. Also, I see no reason to bother calling mountpoint; either umount && rmdir or just umount and then rmdir, in the latter case knowing that rmdir is safe because if the umount failed, it can’t do anything.
(In reply to Christopher Head from comment #4) > Better would be rmdir; rm doesn’t remove directories at all (even if empty) > unless -r is provided. Also, I see no reason to bother calling mountpoint; > either umount && rmdir or just umount and then rmdir, in the latter case > knowing that rmdir is safe because if the umount failed, it can’t do > anything. First, I thin the same, But then I remember that, there can be a cases, when, for some reason, mountpoint-directory can be unclean when [system] mounting on it, so it will be non-empty after unmount, so rmdir will not remove it. That is why rm -r can be needed. Next part, is that it `umount` is not obliged to always exit with 0 on success and with 1 on failure. While mountpoint -q *is* obliged (but have a bit opposite meaning).
(In reply to Vadim A. Misbakh-Soloviov (mva) from comment #5) > First, I thin the same, But then I remember that, there can be a cases, > when, for some reason, mountpoint-directory can be unclean when [system] > mounting on it, so it will be non-empty after unmount, so rmdir will not > remove it. That’s impossible. The directory is created a couple of lines earlier with “mktemp -d” and nothing is put inside it. It is guaranteed to be empty if the umount is successful.
The point of clean_run was to make sure that the directory, "/run" doesn't have any contents. That is why it bind mounts it to the location in /tmp then removes the contents of that location. "/run" is a mount point and there should be no contents in that directory. If I shouldn't care about that, I can remove this function completely.
(In reply to William Hubbs from comment #7) > The point of clean_run was to make sure that the directory, "/run" doesn't > have any contents. That is why it bind mounts it to the location in /tmp > then removes the contents of that location. "/run" is a mount point and > there should be no contents in that directory. > > If I shouldn't care about that, I can remove this function completely. It’s not the “rm -rf $dir/run/*” that I was talking about. I understand that’s meant to clean the /run directory of the underlying disk. I don’t know whether or not that’s important to do; cleaning that directory doesn’t sound particularly crazy to me. The thing I’m pointing out is the “rm -rf $dir” two lines below, which is meant to just remove the temporary directory after the bind-mount is unmounted. That’s really dangerous if the umount fails.
This is fixed in commit 1558ad2.
