| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAK7LNAR2xDjbn+BZqUrgbDxPJUyQBULFB51kTiN8Nc78DXVyEw@mail.gmail.com>
Date: Sun, 28 Jan 2024 16:32:33 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Jose Ignacio Tornos Martinez <jtornosm@...hat.com>
Cc: dcavalca@...a.com, linux-kbuild@...r.kernel.org,
linux-kernel@...r.kernel.org, nathan@...nel.org, ndesaulniers@...gle.com,
nicolas@...sle.eu, stable@...r.kernel.org
Subject: Re: [PATCH] rpm-pkg: simplify installkernel %post
On Tue, Jan 23, 2024 at 3:23 AM Jose Ignacio Tornos Martinez
<jtornosm@...hat.com> wrote:
>
> The new installkernel application that is now included in systemd-udev
> package allows installation although destination files are already present
> in the boot directory of the kernel package, but is failing with the
> implemented workaround for the old installkernel application from grubby
> package.
>
> For the new installkernel application, as Davide says:
> <<The %post currently does a shuffling dance before calling installkernel.
> This isn't actually necessary afaict, and the current implementation
> ends up triggering downstream issues such as
> https://github.com/systemd/systemd/issues/29568
> This commit simplifies the logic to remove the shuffling. For reference,
> the original logic was added in commit 3c9c7a14b627("rpm-pkg: add %post
> section to create initramfs and grub hooks").>>
>
> But we need to keep the old behavior as well, because the old installkernel
> application from grubby package, does not allow this simplification and
> we need to be backward compatible to avoid issues with the different
> packages.
>
> Mimic Fedora shipping process and store vmlinuz, config amd System.map
> in the module directory instead of the boot directory. In this way, we will
> avoid the commented problem for all the cases, because the new destination
> files are not going to exist in the boot directory of the kernel package.
>
> Replace installkernel tool with kernel-install tool, because the latter is
> more complete.
>
> Besides, after installkernel tool execution, check to complete if the
> correct package files vmlinuz, System.map and config files are present
> in /boot directory, and if necessary, copy manually for install operation.
> In this way, take into account if files were not previously copied from
> /usr/lib/kernel/install.d/* scripts and if the suitable files for the
> requested package are present (it could be others if the rpm files were
> replace with a new pacakge with the same release and a different build).
>
> Tested with Fedora 38, Fedora 39, RHEL 9, Oracle Linux 9.3,
> openSUSE Tumbleweed and openMandrive ROME, using dnf/zypper and rpm tools.
>
> cc: stable@...r.kernel.org
> Co-Developed-by: Davide Cavalca <dcavalca@...a.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@...hat.com>
> ---
> V1 -> V2:
> - Complete to be backward compatible with the previous installkernel
> application.
> V2 -> V3:
> - Follow the suggestions from Masahiro Yamada and change the installation
> destination to avoid problems instead of checking the package.
> V3 -> V4:
> - Make the patch applicable to linux-kbuild/for-next (ia64 support was
> already removed).
> V4 -> V5:
> - Complete for other Linux distributions.
> V5 -> V6
> - Simplify and do more compatible checks when copied files wants to be
> replaced.
> - Remove %preun because it will be better done with another patch.
> - Add indentation and quotation
>
> scripts/package/kernel.spec | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/package/kernel.spec b/scripts/package/kernel.spec
> index 89298983a169..0bff257ec3d4 100644
> --- a/scripts/package/kernel.spec
> +++ b/scripts/package/kernel.spec
> @@ -55,12 +55,12 @@ patch -p1 < %{SOURCE2}
> %{make} %{makeflags} KERNELRELEASE=%{KERNELRELEASE} KBUILD_BUILD_VERSION=%{release}
>
> %install
> -mkdir -p %{buildroot}/boot
> -cp $(%{make} %{makeflags} -s image_name) %{buildroot}/boot/vmlinuz-%{KERNELRELEASE}
> +mkdir -p %{buildroot}/lib/modules/%{KERNELRELEASE}
> +cp $(%{make} %{makeflags} -s image_name) %{buildroot}/lib/modules/%{KERNELRELEASE}/vmlinuz
> %{make} %{makeflags} INSTALL_MOD_PATH=%{buildroot} modules_install
> %{make} %{makeflags} INSTALL_HDR_PATH=%{buildroot}/usr headers_install
> -cp System.map %{buildroot}/boot/System.map-%{KERNELRELEASE}
> -cp .config %{buildroot}/boot/config-%{KERNELRELEASE}
> +cp System.map %{buildroot}/lib/modules/%{KERNELRELEASE}
> +cp .config %{buildroot}/lib/modules/%{KERNELRELEASE}/config
> ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEASE}/build
> %if %{with_devel}
> %{make} %{makeflags} run-command KBUILD_RUN_COMMAND='${srctree}/scripts/package/install-extmod-build %{buildroot}/usr/src/kernels/%{KERNELRELEASE}'
> @@ -70,31 +70,31 @@ ln -fns /usr/src/kernels/%{KERNELRELEASE} %{buildroot}/lib/modules/%{KERNELRELEA
> rm -rf %{buildroot}
>
> %post
> -if [ -x /sbin/installkernel -a -r /boot/vmlinuz-%{KERNELRELEASE} -a -r /boot/System.map-%{KERNELRELEASE} ]; then
> -cp /boot/vmlinuz-%{KERNELRELEASE} /boot/.vmlinuz-%{KERNELRELEASE}-rpm
> -cp /boot/System.map-%{KERNELRELEASE} /boot/.System.map-%{KERNELRELEASE}-rpm
> -rm -f /boot/vmlinuz-%{KERNELRELEASE} /boot/System.map-%{KERNELRELEASE}
> -/sbin/installkernel %{KERNELRELEASE} /boot/.vmlinuz-%{KERNELRELEASE}-rpm /boot/.System.map-%{KERNELRELEASE}-rpm
> -rm -f /boot/.vmlinuz-%{KERNELRELEASE}-rpm /boot/.System.map-%{KERNELRELEASE}-rpm
> +if [ -x /usr/bin/kernel-install ]; then
> + /usr/bin/kernel-install add %{KERNELRELEASE} /lib/modules/%{KERNELRELEASE}/vmlinuz
> fi
> +for file in vmlinuz System.map config; do
> + if [ ! -e "/boot/${file}-%{KERNELRELEASE}" ] || ! cmp --silent "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"; then
> + cp "/lib/modules/%{KERNELRELEASE}/${file}" "/boot/${file}-%{KERNELRELEASE}"
> + fi
> +done
I am fine with this approach, but
[ ! -e "/boot/${file}-%{KERNELRELEASE}" ]
is a redundant check now.
When the file does not exist, "cmp --silent" exits
with 2 instead of 1.
Anyway, "cmp --silent" fails.
You can simplify the conditional to:
if ! cmp --silent "/lib/modules/%{KERNELRELEASE}/${file}"
"/boot/${file}-%{KERNELRELEASE}"; then
> %preun
> if [ -x /sbin/new-kernel-pkg ]; then
> -new-kernel-pkg --remove %{KERNELRELEASE} --rminitrd --initrdfile=/boot/initramfs-%{KERNELRELEASE}.img
> + new-kernel-pkg --remove %{KERNELRELEASE} --rminitrd --initrdfile=/boot/initramfs-%{KERNELRELEASE}.img
This is a noise change.
Please drop.
> elif [ -x /usr/bin/kernel-install ]; then
> -kernel-install remove %{KERNELRELEASE}
> + /usr/bin/kernel-install remove %{KERNELRELEASE}
> fi
>
> %postun
> if [ -x /sbin/update-bootloader ]; then
> -/sbin/update-bootloader --remove %{KERNELRELEASE}
> + /sbin/update-bootloader --remove %{KERNELRELEASE}
Ditto.
Please create a separate patch if you change the coding style.
But, rather, I am thinking of reverting
6ef41e22a320 and 27c3bffd230ab
> fi
>
> %files
> %defattr (-, root, root)
> /lib/modules/%{KERNELRELEASE}
> %exclude /lib/modules/%{KERNELRELEASE}/build
> -/boot/*
>
> %files headers
> %defattr (-, root, root)
> --
> 2.43.0
>
--
Best Regards
Masahiro Yamada
Powered by blists - more mailing lists