From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 1616D138334 for ; Mon, 9 Dec 2019 10:42:48 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id D754BE0867; Mon, 9 Dec 2019 10:42:43 +0000 (UTC) Received: from othala.iewc.co.za (othala.iewc.co.za [154.73.34.78]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 16001E0858 for ; Mon, 9 Dec 2019 10:42:42 +0000 (UTC) Received: from [165.16.203.58] (helo=tauri.local.uls.co.za) by othala.iewc.co.za with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.2) (envelope-from ) id 1ieGUt-0006Py-W5; Mon, 09 Dec 2019 12:42:36 +0200 Received: from plastiekpoot.dhcp.uls.co.za ([192.168.42.199]) by tauri.local.uls.co.za with esmtp (Exim 4.92.2) (envelope-from ) id 1ieGUs-0000OK-RY; Mon, 09 Dec 2019 12:42:35 +0200 Subject: Re: [gentoo-dev] [PATCH v4] mount-boot.eclass: Check if /boot is sane, but don't try to mount it. To: gentoo-dev@lists.gentoo.org, =?UTF-8?Q?Ulrich_M=c3=bcller?= References: From: Jaco Kroon Autocrypt: addr=jaco@uls.co.za; prefer-encrypt=mutual; keydata= mQENBFXtplYBCADM6RTLCOSPiclevkn/gdf8h9l+kKA6N+WGIIFuUtoc9Gaf8QhXWW/fvUq2 a3eo4ULVFT1jJ56Vfm4MssGA97NZtlOe3cg8QJMZZhsoN5wetG9SrJvT9Rlltwo5nFmXY3ZY gXsdwkpDr9Y5TqBizx7DGxMd/mrOfXeql57FWFeOc2GuJBnHPZQMJsQ66l2obPn36hWEtHYN gcUSPH3OOusSEGZg/oX/8WSDQ/b8xz1JKTEgcnu/JR0FxzjY19zSHmbnyVU+/gF3oeJFcEUk HvZu776LRVdcZ0lb1bHQB2K9rTZBVeZLitgAefPVH2uERVSO8EZO1I5M7afV0Kd/Vyn9ABEB AAG0G0phY28gS3Jvb24gPGphY29AdWxzLmNvLnphPokBNwQTAQgAIQUCVe2mVgIbAwULCQgH AgYVCAkKCwIEFgIDAQIeAQIXgAAKCRAILcSxr/fungCPB/sHrfufpRbrVTtHUjpbY4bTQLQE bVrh4/yMiKprALRYy0nsMivl16Q/3rNWXJuQ0gR/faC3yNlDgtEoXx8noXOhva9GGHPGTaPT hhpcp/1E4C9Ghcaxw3MRapVnSKnSYL+zOOpkGwye2+fbqwCkCYCM7Vu6ws3+pMzJNFK/UOgW Tj8O5eBa3DiU4U26/jUHEIg74U+ypYPcj5qXG0xNXmmoDpZweW41Cfo6FMmgjQBTEGzo9e5R kjc7MH3+IyJvP4bzE5Paq0q0b5zZ8DUJFtT7pVb3FQTz1v3CutLlF1elFZzd9sZrg+mLA5PM o8PG9FLw9ZtTE314vgMWJ+TTYX0kuQENBFXtplYBCADedX9HSSJozh4YIBT+PuLWCTJRLTLu jXU7HobdK1EljPAi1ahCUXJR+NHvpJLSq/N5rtL12ejJJ4EMMp2UUK0IHz4kx26FeAJuOQMe GEzoEkiiR15ufkApBCRssIj5B8OA/351Y9PFore5KJzQf1psrCnMSZoJ89KLfU7C5S+ooX9e re2aWgu5jqKgKDLa07/UVHyxDTtQKRZSFibFCHbMELYKDr3tUdUfCDqVjipCzHmLZ+xMisfn yX9aTVI3FUIs8UiqM5xlxqfuCnDrKBJjQs3uvmd6cyhPRmnsjase48RoO84Ckjbp/HVu0+1+ 6vgiPjbe4xk7Ehkw1mfSxb79ABEBAAGJAR8EGAEIAAkFAlXtplYCGwwACgkQCC3Esa/37p7u XwgAjpFzUj+GMmo8ZeYwHH6YfNZQV+hfesr7tqlZn5DhQXJgT2NF6qh5Vn8TcFPR4JZiVIkF o0je7c8FJe34Aqex/H9R8LxvhENX/YOtq5+PqZj59y9G9+0FFZ1CyguTDC845zuJnnR5A0lw FARZaL8T7e6UGphtiT0NdR7EXnJ/alvtsnsNudtvFnKtigYvtw2wthW6CLvwrFjsuiXPjVUX 825zQUnBHnrED6vG67UG4z5cQ4uY/LcSNsqBsoj6/wsT0pnqdibhCWmgFimOsSRgaF7qsVtg TWyQDTjH643+qYbJJdH91LASRLrenRCgpCXgzNWAMX6PJlqLrNX1Ye4CQw== Organization: Ultimate Linux Solutions (Pty) Ltd Message-ID: <9a9f76ee-56aa-e566-a539-bdb07d82501b@uls.co.za> Date: Mon, 9 Dec 2019 12:42:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Spam-report: Relay access (othala.iewc.co.za). X-Archives-Salt: 7d593d50-dde1-4458-8395-2c4c7b2fcf83 X-Archives-Hash: 1fc956a794490770c748dc31bc6d5059 Hi Ulrich, I'm happy with this "as is", but there may be a few improvements still. By the way:  This improves the situation for mounted ro /boot by moving the check from preinst to pretend. For noauto /boot (I believe the default and recommended) this fixes things. This is the reason I decided to rather go with mounting /boot but as ro instead of not mounting at all. May I also suggest we start recommended read-only /boot instead of not mounted at all in order to avoid similar issues from recurring? Kind Regards, Jaco On 2019/12/07 11:10, Ulrich Müller wrote: > 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 Acked-by: Jaco Kroon > > --- > v3: Exit awk commands on first match. > > v4: Added die statements after awk commands >     Fixed typo in mount-boot_is_disabled function documentation >     Reverted renaming of I_KNOW_WHAT_I_AM_DOING variable > >  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 938df6732f43..ca27aca7efbd 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 > +# @FUNCTION: mount-boot_is_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 I don't use spaces in path names ... but what happens here if ROOT or EPREFIX (and by implication EROOT) contains a space? What about just checking "${EROOT}/boot" instead? Would that even be possible ... ? > >   >      # 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 >          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 || die "awk failed") > + > +    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 || die "awk failed") >   > -    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() { :; }