Mailing List Archive

[PATCH v3] mount-boot.eclass: Check if /boot is sane, but don't try to mount it.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The eclass failed to remount a read-only mounted /boot, because package
collision sanity checks in recent Portage versions prevented it from
reaching pkg_preinst() at all. Furthermore, with the "mount-sandbox"
feature enabled, the mount won't be propagated past pkg_preinst() and
installed files would end up under the (shadowed) mount point.

Therefore don't even attempt to mount /boot ourselves, but error out
if it isn't mounted read/write and ask the user to mount /boot.

Also clean up and simplify. (For example, awk is a grown-up program
which doesn't need any help from egrep or sed. :-)

Closes: https://bugs.gentoo.org/532264
See-also: https://bugs.gentoo.org/274130#c5
Signed-off-by: Ulrich Müller <ulm@gentoo.org>
---
v3: Exit awk commands on first match.

eclass/mount-boot.eclass | 144 +++++++++++++--------------------------
1 file changed, 48 insertions(+), 96 deletions(-)

diff --git a/eclass/mount-boot.eclass b/eclass/mount-boot.eclass
index 938df6732f4..a7d8851aa6e 100644
--- a/eclass/mount-boot.eclass
+++ b/eclass/mount-boot.eclass
@@ -1,156 +1,108 @@
-# Copyright 1999-2015 Gentoo Foundation
+# Copyright 1999-2019 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

# @ECLASS: mount-boot.eclass
# @MAINTAINER:
# base-system@gentoo.org
# @BLURB: functions for packages that install files into /boot
# @DESCRIPTION:
# This eclass is really only useful for bootloaders.
#
# If the live system has a separate /boot partition configured, then this
# function tries to ensure that it's mounted in rw mode, exiting with an
-# error if it can't. It does nothing if /boot isn't a separate partition.
+# error if it can't. It does nothing if /boot isn't a separate partition.
+
+case ${EAPI:-0} in
+ 4|5|6|7) ;;
+ *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
+esac

EXPORT_FUNCTIONS pkg_pretend pkg_preinst pkg_postinst pkg_prerm pkg_postrm

# @FUNCTION: mount-boot_disabled
# @INTERNAL
# @DESCRIPTION:
# Detect whether the current environment/build settings are such that we do not
# want to mess with any mounts.
mount-boot_is_disabled() {
- # Since this eclass only deals with /boot, skip things when ROOT is active.
- if [[ "${ROOT:-/}" != "/" ]] ; then
+ # Since this eclass only deals with /boot, skip things when EROOT is active.
+ if [[ ${EROOT:-/} != / ]] ; then
return 0
fi

# If we're only building a package, then there's no need to check things.
- if [[ "${MERGE_TYPE}" == "buildonly" ]] ; then
+ if [[ ${MERGE_TYPE} == buildonly ]] ; then
return 0
fi

# The user wants us to leave things be.
- if [[ -n ${DONT_MOUNT_BOOT} ]] ; then
+ if [[ -n ${I_KNOW_WHAT_I_AM_DOING} ]] ; then
return 0
fi

# OK, we want to handle things ourselves.
return 1
}

# @FUNCTION: mount-boot_check_status
# @INTERNAL
# @DESCRIPTION:
-# Figure out what kind of work we need to do in order to have /boot be sane.
-# Return values are:
-# 0 - Do nothing at all!
-# 1 - It's mounted, but is currently ro, so need to remount rw.
-# 2 - It's not mounted, so need to mount it rw.
+# Check if /boot is sane, i.e., mounted read/write if on a separate
+# partition. Die if conditions are not fulfilled.
mount-boot_check_status() {
# Get out fast if possible.
- mount-boot_is_disabled && return 0
+ mount-boot_is_disabled && return

# note that /dev/BOOT is in the Gentoo default /etc/fstab file
- local fstabstate=$(awk '!/^#|^[[:blank:]]+#|^\/dev\/BOOT/ {print $2}' /etc/fstab | egrep "^/boot$" )
- local procstate=$(awk '$2 ~ /^\/boot$/ {print $2}' /proc/mounts)
- local proc_ro=$(awk '{ print $2 " ," $4 "," }' /proc/mounts | sed -n '/^\/boot .*,ro,/p')
-
- if [ -n "${fstabstate}" ] && [ -n "${procstate}" ] ; then
- if [ -n "${proc_ro}" ] ; then
- echo
- einfo "Your boot partition, detected as being mounted at /boot, is read-only."
- einfo "It will be remounted in read-write mode temporarily."
- return 1
- else
- echo
- einfo "Your boot partition was detected as being mounted at /boot."
- einfo "Files will be installed there for ${PN} to function correctly."
- return 0
- fi
- elif [ -n "${fstabstate}" ] && [ -z "${procstate}" ] ; then
- echo
- einfo "Your boot partition was not mounted at /boot, so it will be automounted for you."
- einfo "Files will be installed there for ${PN} to function correctly."
- return 2
- else
- echo
+ local fstabstate=$(awk '!/^[[:blank:]]*#|^\/dev\/BOOT/ && $2 == "/boot" \
+ { print 1; exit }' /etc/fstab)
+
+ if [[ -z ${fstabstate} ]] ; then
einfo "Assuming you do not have a separate /boot partition."
- return 0
+ return
fi
-}

-mount-boot_pkg_pretend() {
- # Get out fast if possible.
- mount-boot_is_disabled && return 0
+ local procstate=$(awk '$2 == "/boot" \
+ { print gensub(/^(.*,)?(ro|rw)(,.*)?$/, "\\2", 1, $4); exit }' \
+ /proc/mounts)

- elog "To avoid automounting and auto(un)installing with /boot,"
- elog "just export the DONT_MOUNT_BOOT variable."
- mount-boot_check_status
+ if [[ -z ${procstate} ]] ; then
+ eerror "Your boot partition is not mounted at /boot."
+ eerror "Please mount it and retry."
+ die "/boot not mounted"
+ fi
+
+ if [[ ${procstate} == ro ]] ; then
+ eerror "Your boot partition, detected as being mounted at /boot," \
+ "is read-only."
+ eerror "Please remount it read/write and retry."
+ die "/boot mounted read-only"
+ fi
+
+ einfo "Your boot partition was detected as being mounted at /boot."
+ einfo "Files will be installed there for ${PN} to function correctly."
}

-mount-boot_mount_boot_partition() {
+mount-boot_pkg_pretend() {
mount-boot_check_status
- case $? in
- 0) # Nothing to do.
- ;;
- 1) # Remount it rw.
- mount -o remount,rw /boot
- if [ $? -ne 0 ] ; then
- echo
- eerror "Unable to remount in rw mode. Please do it manually!"
- die "Can't remount in rw mode. Please do it manually!"
- fi
- touch /boot/.e.remount
- ;;
- 2) # Mount it rw.
- mount /boot -o rw
- if [ $? -ne 0 ] ; then
- echo
- eerror "Cannot automatically mount your /boot partition."
- eerror "Your boot partition has to be mounted rw before the installation"
- eerror "can continue. ${PN} needs to install important files there."
- die "Please mount your /boot partition manually!"
- fi
- touch /boot/.e.mount
- ;;
- esac
}

mount-boot_pkg_preinst() {
- # Handle older EAPIs.
- case ${EAPI:-0} in
- [0-3]) mount-boot_pkg_pretend ;;
- esac
-
- mount-boot_mount_boot_partition
+ mount-boot_check_status
}

mount-boot_pkg_prerm() {
- touch "${ROOT}"/boot/.keep 2>/dev/null
- mount-boot_mount_boot_partition
- touch "${ROOT}"/boot/.keep 2>/dev/null
-}
+ mount-boot_check_status

-mount-boot_umount_boot_partition() {
- # Get out fast if possible.
- mount-boot_is_disabled && return 0
-
- if [ -e /boot/.e.remount ] ; then
- einfo "Automatically remounting /boot as ro as it was previously."
- rm -f /boot/.e.remount
- mount -o remount,ro /boot
- elif [ -e /boot/.e.mount ] ; then
- einfo "Automatically unmounting /boot as it was previously."
- rm -f /boot/.e.mount
- umount /boot
+ if [[ -z ${EPREFIX} ]] \
+ && ! ( shopt -s failglob; : "${EROOT}"/boot/.keep* ) 2>/dev/null
+ then
+ # Create a .keep file, in case it is shadowed at the mount point
+ touch "${EROOT}"/boot/.keep 2>/dev/null
fi
}

-mount-boot_pkg_postinst() {
- mount-boot_umount_boot_partition
-}
+# No-op phases for backwards compatibility
+mount-boot_pkg_postinst() { :; }

-mount-boot_pkg_postrm() {
- mount-boot_umount_boot_partition
-}
+mount-boot_pkg_postrm() { :; }
--
2.24.0
Re: [PATCH v3] mount-boot.eclass: Check if /boot is sane, but don't try to mount it. [ In reply to ]
On Fri, 06 Dec 2019 16:35:53 +0100
Ulrich Müller <ulm@gentoo.org> wrote:

> # The user wants us to leave things be.
> - if [[ -n ${DONT_MOUNT_BOOT} ]] ; then
> + if [[ -n ${I_KNOW_WHAT_I_AM_DOING} ]] ; then
> return 0
> fi

The rest of patch looks ok but I find I_KNOW_WHAT_I_AM_DOING
proliferation worrying. Having enough eclasses guard things on it I don't
really know what I am doing :)

For example developer profile sets it on by default and disables perl error
checks. I don't think it's intentional.

I suggest giving this variable a unique specific name. And phase out
${I_KNOW_WHAT_I_AM_DOING} uses from tree completely.

--

Sergei
Re: [PATCH v3] mount-boot.eclass: Check if /boot is sane, but don't try to mount it. [ In reply to ]
>>>>> On Sat, 07 Dec 2019, Sergei Trofimovich wrote:

>> # The user wants us to leave things be.
>> - if [[ -n ${DONT_MOUNT_BOOT} ]] ; then
>> + if [[ -n ${I_KNOW_WHAT_I_AM_DOING} ]] ; then
>> return 0
>> fi

> The rest of patch looks ok but I find I_KNOW_WHAT_I_AM_DOING
> proliferation worrying. Having enough eclasses guard things on it I
> don't really know what I am doing :)

> For example developer profile sets it on by default and disables perl
> error checks. I don't think it's intentional.

Oh, I forgot that it is set globally in the developer profile (and I
think that's a stupid idea). Indeed we should use a different variable
then.

> I suggest giving this variable a unique specific name.

Would it be acceptable to leave DONT_MOUNT_BOOT in place? It would have
the advantage that users won't have to update their config.

> And phase out ${I_KNOW_WHAT_I_AM_DOING} uses from tree completely.

That's a separate discussion.

Ulrich
Re: [PATCH v3] mount-boot.eclass: Check if /boot is sane, but don't try to mount it. [ In reply to ]
On Sat, 07 Dec 2019 06:44:21 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sat, 07 Dec 2019, Sergei Trofimovich wrote:
>
> >> # The user wants us to leave things be.
> >> - if [[ -n ${DONT_MOUNT_BOOT} ]] ; then
> >> + if [[ -n ${I_KNOW_WHAT_I_AM_DOING} ]] ; then
> >> return 0
> >> fi
>
> > The rest of patch looks ok but I find I_KNOW_WHAT_I_AM_DOING
> > proliferation worrying. Having enough eclasses guard things on it I
> > don't really know what I am doing :)
>
> > For example developer profile sets it on by default and disables perl
> > error checks. I don't think it's intentional.
>
> Oh, I forgot that it is set globally in the developer profile (and I
> think that's a stupid idea). Indeed we should use a different variable
> then.
>
> > I suggest giving this variable a unique specific name.
>
> Would it be acceptable to leave DONT_MOUNT_BOOT in place? It would have
> the advantage that users won't have to update their config.

Sounds good.

> > And phase out ${I_KNOW_WHAT_I_AM_DOING} uses from tree completely.
>
> That's a separate discussion.

Sure. As long as we don't add extra uses.


--

Sergei