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 33001138334 for ; Fri, 6 Dec 2019 14:03:53 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 72B8BE08D3; Fri, 6 Dec 2019 14:03:49 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (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 20ECAE08AC for ; Fri, 6 Dec 2019 14:03:49 +0000 (UTC) Received: from pomiot (c134-66.icpnet.pl [85.221.134.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: mgorny) by smtp.gentoo.org (Postfix) with ESMTPSA id 641FD34D84B; Fri, 6 Dec 2019 14:03:47 +0000 (UTC) Message-ID: Subject: Re: [gentoo-dev] [PATCH] mount-boot.eclass: Check if /boot is sane, but don't try to mount it. From: =?UTF-8?Q?Micha=C5=82_G=C3=B3rny?= To: gentoo-dev@lists.gentoo.org Date: Fri, 06 Dec 2019 15:03:42 +0100 In-Reply-To: References: Organization: Gentoo Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-TiOILCJt/ydIFnRPcm9L" User-Agent: Evolution 3.32.4 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 X-Archives-Salt: fe8dcecc-fe3a-4dbc-8a0f-e15dd69cd991 X-Archives-Hash: e54bc3762e9b56595ea355699579dbd8 --=-TiOILCJt/ydIFnRPcm9L Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2019-12-06 at 14:11 +0100, Ulrich M=C3=BCller 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_pretend() at all. Furthermore, with the "mount-sandbox" Did you mean: pkg_preinst? > feature enabled, the mount won't be propagated past pkg_preinst() and > installed files would end up under the (shadowed) mount point. >=20 > 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. >=20 > Also clean up and simplify. (For example, awk is a grown-up program > which doesn't need any help from egrep or sed. :-) >=20 > Closes: https://bugs.gentoo.org/532264 > Signed-off-by: Ulrich M=C3=BCller > --- > eclass/mount-boot.eclass | 137 ++++++++++++--------------------------- > 1 file changed, 43 insertions(+), 94 deletions(-) >=20 > diff --git a/eclass/mount-boot.eclass b/eclass/mount-boot.eclass > index 938df6732f4..1d7eb8bfc29 100644 > --- a/eclass/mount-boot.eclass > +++ b/eclass/mount-boot.eclass > @@ -1,156 +1,105 @@ > -# Copyright 1999-2015 Gentoo Foundation > +# Copyright 1999-2019 Gentoo Authors > # Distributed under the terms of the GNU General Public License v2 > =20 > # @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 thi= s > # 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 partitio= n. > + > +case ${EAPI:-0} in > + 4|5|6|7) ;; > + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;; > +esac > =20 > EXPORT_FUNCTIONS pkg_pretend pkg_preinst pkg_postinst pkg_prerm pkg_post= rm > =20 > # @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 act= ive. > - if [[ "${ROOT:-/}" !=3D "/" ]] ; then > + if [[ ${ROOT:-/} !=3D "/" ]] ; then I suppose you can unquote RHS too since it doesn't contain any pattern characters, if you are already touching quoting. > return 0 > fi > =20 > # If we're only building a package, then there's no need to check thing= s. > - if [[ "${MERGE_TYPE}" =3D=3D "buildonly" ]] ; then > + if [[ ${MERGE_TYPE} =3D=3D "buildonly" ]] ; then > return 0 > fi > =20 > # 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 > =20 > # OK, we want to handle things ourselves. > return 1 > } > =20 > # @FUNCTION: mount-boot_check_status > # @INTERNAL > # @DESCRIPTION: > -# Figure out what kind of work we need to do in order to have /boot be s= ane. > -# 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. Return 0 if conditions are fulfilled, otherwise die. I don't think there's a point in explicitly defining the return value if there is no alternative. > mount-boot_check_status() { > # Get out fast if possible. > mount-boot_is_disabled && return 0 > =20 > # note that /dev/BOOT is in the Gentoo default /etc/fstab file > - local fstabstate=3D$(awk '!/^#|^[[:blank:]]+#|^\/dev\/BOOT/ {print $2}'= /etc/fstab | egrep "^/boot$" ) > - local procstate=3D$(awk '$2 ~ /^\/boot$/ {print $2}' /proc/mounts) > - local proc_ro=3D$(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 re= ad-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 aut= omounted for you." > - einfo "Files will be installed there for ${PN} to function correctly." > - return 2 > - else > - echo > + local fstabstate=3D$(awk '!/^[[:blank:]]*#|^\/dev\/BOOT/ && $2 =3D=3D "= /boot" \ > + {print $2}' /etc/fstab) The 'print' here is used as a boolean... why not use a boolean output instead? > + > + if [[ -z ${fstabstate} ]] ; then > einfo "Assuming you do not have a separate /boot partition." > return 0 > fi > -} > =20 > -mount-boot_pkg_pretend() { > - # Get out fast if possible. > - mount-boot_is_disabled && return 0 > + local procstate=3D$(awk '$2 =3D=3D "/boot" \ > + {print gensub(/^(.*,)?(ro|rw)(,.*)?$/, "\\2", 1, $4)}' /proc/mounts) Shouldn't this use /proc/self/mounts? > =20 > - 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} =3D=3D "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." > + return 0 > } > =20 > -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 installat= ion" > - eerror "can continue. ${PN} needs to install important files there." > - die "Please mount your /boot partition manually!" > - fi > - touch /boot/.e.mount > - ;; > - esac > } > =20 > 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 > } > =20 > 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_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 > + mount-boot_check_status > + if ! ( shopt -s failglob; : "${ROOT}"/boot/.keep* ) 2>/dev/null ; then EROOT? > + # Create a .keep file, in case it is shadowed at the mount point > + touch "${ROOT}"/boot/.keep 2>/dev/null > fi > } > =20 > -mount-boot_pkg_postinst() { > - mount-boot_umount_boot_partition > -} > +# No-op phases for backwards compatibility > +mount-boot_pkg_postinst() { :; } > =20 > -mount-boot_pkg_postrm() { > - mount-boot_umount_boot_partition > -} > +mount-boot_pkg_postrm() { :; } --=20 Best regards, Micha=C5=82 G=C3=B3rny --=-TiOILCJt/ydIFnRPcm9L Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQGTBAABCgB9FiEEx2qEUJQJjSjMiybFY5ra4jKeJA4FAl3qX79fFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEM3 NkE4NDUwOTQwOThEMjhDQzhCMjZDNTYzOUFEQUUyMzI5RTI0MEUACgkQY5ra4jKe JA7ZHQgAvmgSQWeXof3lNnvTfa1BmHqx1yAE6l+B+uA+iaC+pXesmhM7rWWGjpl7 xdGAC3g65ls55oRpY+Jm2t1MJINYOQJzzf2h0TZBfnYXbKe+5XGssMEPQJw7mkWh UjeoHaTRrtvokiqKEDF0dW5okuAY0YHXAysX09LUYnPyFdCIJ+7Pm2Q39rdgo93L wy38NfYd8guZP+KBm/kAhIIA/5ZkKTs5Gk5uQGR2EWhuSBhqPJgLvDiVPkiFVqSO Ld5lfXKi7ChphUbaA68j2jsrCVw3WRHLW0TVgHzUsHrc/AsjPNcWpbnlmfBKOLlV J6nbeB77cb9kIniO2RVFzBvX7CTVJQ== =Obd9 -----END PGP SIGNATURE----- --=-TiOILCJt/ydIFnRPcm9L--