diff mbox series

classes/kernel: Delay module signing until after strip

Message ID bc15a9ebae0ad3841ac2be887c288b451426e69b.1720185337.git.joerg.sommer@navimatix.de
State New
Headers show
Series classes/kernel: Delay module signing until after strip | expand

Commit Message

Jörg Sommer July 5, 2024, 1:19 p.m. UTC 
From: Jörg Sommer <joerg.sommer@navimatix.de>

The current do_package does not strip the kernel modules, if they are
signed. Hence, the files in a release image stay very big and the debug
information are not split into the kernel-dbg package.

The idea of this change is to suppress the signing of the modules on
`modules_install` and run `modules_sign` after the debug information was
striped.

Signed-off-by: Jörg Sommer <joerg.sommer@navimatix.de>
---
 meta/classes-recipe/kernel.bbclass | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)


Another idea would be to move the code to split the debug info in package.bbclass to a
stand-alone tool and inject this in the kernel's modules_install.

Comments

Bruce Ashfield July 5, 2024, 2:08 p.m. UTC | #1 
On Fri, Jul 5, 2024 at 9:19 AM Jörg Sommer via lists.openembedded.org
<joerg.sommer=navimatix.de@lists.openembedded.org> wrote:
>
> From: Jörg Sommer <joerg.sommer@navimatix.de>
>
> The current do_package does not strip the kernel modules, if they are
> signed. Hence, the files in a release image stay very big and the debug
> information are not split into the kernel-dbg package.
>
> The idea of this change is to suppress the signing of the modules on
> `modules_install` and run `modules_sign` after the debug information was
> striped.
>
> Signed-off-by: Jörg Sommer <joerg.sommer@navimatix.de>
> ---
>  meta/classes-recipe/kernel.bbclass | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
>
> Another idea would be to move the code to split the debug info in package.bbclass to a
> stand-alone tool and inject this in the kernel's modules_install.
>
>
> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> index 89badd90f1..96e40be085 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -461,7 +461,8 @@ kernel_do_install() {
>         #
>         unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
>         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> -               oe_runmake DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> +               # don't sign now, it's down after extraction of debug information
> +               oe_runmake CONFIG_MODULE_SIG_ALL=n DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install

This isn't a good idea, since it both hardcodes the option name, and
it doesn't allow any method of overriding the setting.

>                 rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/build"
>                 rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/source"
>                 # Remove empty module directories to prevent QA issues
> @@ -497,6 +498,16 @@ kernel_do_install() {
>         ! [ -e Module.symvers ] || install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
>  }
>
> +sign_kernel_modules() {
> +       unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
> +       if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
> +               # modinst_pre= prevents the cleaning of the target before signing
> +               oe_runmake -C ${B} modinst_pre= DEPMOD=echo MODLIB=${PKGD}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${PKGD}${nonarch_base_libdir}/firmware modules_sign
> +       fi
> +}
> +
> +PACKAGEBUILDPKGD += "sign_kernel_modules"

I've been around for a while, and I've never run across this variable.
According to
my grep it is mostly internal and there are replacement variables that should be
used.

But even if those other variables are used, this seems to force the extra
signing step, which I can say for certain that not everyone wants.

bruce

> +
>  # Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
>  do_kernel_version_sanity_check() {
>         if [ "x${KERNEL_VERSION_SANITY_SKIP}" = "x1" ]; then
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#201607): https://lists.openembedded.org/g/openembedded-core/message/201607
> Mute This Topic: https://lists.openembedded.org/mt/107053401/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Jörg Sommer July 5, 2024, 6:34 p.m. UTC | #2 
Bruce Ashfield via lists.openembedded.org schrieb am Fr 05. Jul, 10:08 (GMT):
> On Fri, Jul 5, 2024 at 9:19 AM Jörg Sommer via lists.openembedded.org
> <joerg.sommer=navimatix.de@lists.openembedded.org> wrote:
> >
> > From: Jörg Sommer <joerg.sommer@navimatix.de>
> >
> > The current do_package does not strip the kernel modules, if they are
> > signed. Hence, the files in a release image stay very big and the debug
> > information are not split into the kernel-dbg package.
> >
> > The idea of this change is to suppress the signing of the modules on
> > `modules_install` and run `modules_sign` after the debug information was
> > striped.
> >
> > Signed-off-by: Jörg Sommer <joerg.sommer@navimatix.de>
> > ---
> >  meta/classes-recipe/kernel.bbclass | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> >
> > Another idea would be to move the code to split the debug info in package.bbclass to a
> > stand-alone tool and inject this in the kernel's modules_install.
> >
> >
> > diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> > index 89badd90f1..96e40be085 100644
> > --- a/meta/classes-recipe/kernel.bbclass
> > +++ b/meta/classes-recipe/kernel.bbclass
> > @@ -461,7 +461,8 @@ kernel_do_install() {
> >         #
> >         unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
> >         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> > -               oe_runmake DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> > +               # don't sign now, it's down after extraction of debug information
> > +               oe_runmake CONFIG_MODULE_SIG_ALL=n DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> 
> This isn't a good idea, since it both hardcodes the option name, and
> it doesn't allow any method of overriding the setting.

The option name is also hardcoded in the kernel Makefile and drives this
feature. I would threat it the same as DEPMOD and MODLIB.

> > +PACKAGEBUILDPKGD += "sign_kernel_modules"

> But even if those other variables are used, this seems to force the extra
> signing step, which I can say for certain that not everyone wants.

What would be the other option? Having an anonymous python function to check
the .config if signing and debug information are enabled and configure the
process accordingly?

As I said, the other way would be to create a stand-alone tool to split the
debug information and feed this as STRIP into modules_install. But this
would alter the process too, because the stripping happens during
do_install.

Jörg
Bruce Ashfield July 5, 2024, 7 p.m. UTC | #3 
On Fri, Jul 5, 2024 at 2:35 PM Jörg Sommer via lists.openembedded.org
<joerg.sommer=navimatix.de@lists.openembedded.org> wrote:
>
> Bruce Ashfield via lists.openembedded.org schrieb am Fr 05. Jul, 10:08 (GMT):
> > On Fri, Jul 5, 2024 at 9:19 AM Jörg Sommer via lists.openembedded.org
> > <joerg.sommer=navimatix.de@lists.openembedded.org> wrote:
> > >
> > > From: Jörg Sommer <joerg.sommer@navimatix.de>
> > >
> > > The current do_package does not strip the kernel modules, if they are
> > > signed. Hence, the files in a release image stay very big and the debug
> > > information are not split into the kernel-dbg package.
> > >
> > > The idea of this change is to suppress the signing of the modules on
> > > `modules_install` and run `modules_sign` after the debug information was
> > > striped.
> > >
> > > Signed-off-by: Jörg Sommer <joerg.sommer@navimatix.de>
> > > ---
> > >  meta/classes-recipe/kernel.bbclass | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > >
> > > Another idea would be to move the code to split the debug info in package.bbclass to a
> > > stand-alone tool and inject this in the kernel's modules_install.
> > >
> > >
> > > diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> > > index 89badd90f1..96e40be085 100644
> > > --- a/meta/classes-recipe/kernel.bbclass
> > > +++ b/meta/classes-recipe/kernel.bbclass
> > > @@ -461,7 +461,8 @@ kernel_do_install() {
> > >         #
> > >         unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
> > >         if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> > > -               oe_runmake DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> > > +               # don't sign now, it's down after extraction of debug information
> > > +               oe_runmake CONFIG_MODULE_SIG_ALL=n DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> >
> > This isn't a good idea, since it both hardcodes the option name, and
> > it doesn't allow any method of overriding the setting.
>
> The option name is also hardcoded in the kernel Makefile and drives this
> feature. I would threat it the same as DEPMOD and MODLIB.

Sure, but that's actually specific to a single kernel and kernel version, and
isn't spread from the kernel source to the meta-data coupling the two.

And I can easily configure that in, or out of, my kernel.

I'd absolutely not treat it the same as those two tools.

>
> > > +PACKAGEBUILDPKGD += "sign_kernel_modules"
>
> > But even if those other variables are used, this seems to force the extra
> > signing step, which I can say for certain that not everyone wants.
>
> What would be the other option? Having an anonymous python function to check
> the .config if signing and debug information are enabled and configure the
> process accordingly?
>

You definitely do NOT want to grep the .config unless absolutely
necessary. That's fundamentally the same binding as putting the
option directly in the command environment.

I'd make it optional via a distro feature that enables signing (or
maybe there already is one for that ? I don't sign modules, so
I'm not familiar with what goes on in that area).

I don't have a specific problem with the implementation, I'm a kernel and
containers maintainer .. not a packaging expert. My issue is that it didn't
look like there was a way to disable the extra processing.

> As I said, the other way would be to create a stand-alone tool to split the
> debug information and feed this as STRIP into modules_install. But this
> would alter the process too, because the stripping happens during
> do_install.

As long as it was optional, I wouldn't be adamant one way or the other.

But if it was passed into STRIP via a variable definition, someone
could set STRIP to whatever value they wanted and avoid the extra
processing or signing.

Bruce

>
> Jörg
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#201614): https://lists.openembedded.org/g/openembedded-core/message/201614
> Mute This Topic: https://lists.openembedded.org/mt/107053401/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Alexandre Belloni July 10, 2024, 8:03 p.m. UTC | #4 
I did give this a go on the autobuilders were it broke all the builds:

https://autobuilder.yoctoproject.org/typhoon/#/builders/73/builds/9148/steps/14/logs/stdio

On 05/07/2024 15:19:08+0200, J�rg Sommer via lists.openembedded.org wrote:
> From: J�rg Sommer <joerg.sommer@navimatix.de>
> 
> The current do_package does not strip the kernel modules, if they are
> signed. Hence, the files in a release image stay very big and the debug
> information are not split into the kernel-dbg package.
> 
> The idea of this change is to suppress the signing of the modules on
> `modules_install` and run `modules_sign` after the debug information was
> striped.
> 
> Signed-off-by: J�rg Sommer <joerg.sommer@navimatix.de>
> ---
>  meta/classes-recipe/kernel.bbclass | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> 
> Another idea would be to move the code to split the debug info in package.bbclass to a
> stand-alone tool and inject this in the kernel's modules_install.
> 
> 
> diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
> index 89badd90f1..96e40be085 100644
> --- a/meta/classes-recipe/kernel.bbclass
> +++ b/meta/classes-recipe/kernel.bbclass
> @@ -461,7 +461,8 @@ kernel_do_install() {
>  	#
>  	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
>  	if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
> -		oe_runmake DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
> +		# don't sign now, it's down after extraction of debug information
> +		oe_runmake CONFIG_MODULE_SIG_ALL=n DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
>  		rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/build"
>  		rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/source"
>  		# Remove empty module directories to prevent QA issues
> @@ -497,6 +498,16 @@ kernel_do_install() {
>  	! [ -e Module.symvers ] || install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
>  }
>  
> +sign_kernel_modules() {
> +	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
> +	if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
> +		# modinst_pre= prevents the cleaning of the target before signing
> +		oe_runmake -C ${B} modinst_pre= DEPMOD=echo MODLIB=${PKGD}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${PKGD}${nonarch_base_libdir}/firmware modules_sign
> +	fi
> +}
> +
> +PACKAGEBUILDPKGD += "sign_kernel_modules"
> +
>  # Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
>  do_kernel_version_sanity_check() {
>  	if [ "x${KERNEL_VERSION_SANITY_SKIP}" = "x1" ]; then
> -- 
> 2.34.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#201607): https://lists.openembedded.org/g/openembedded-core/message/201607
> Mute This Topic: https://lists.openembedded.org/mt/107053401/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index 89badd90f1..96e40be085 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -461,7 +461,8 @@  kernel_do_install() {
 	#
 	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
 	if (grep -q -i -e '^CONFIG_MODULES=y$' .config); then
-		oe_runmake DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
+		# don't sign now, it's down after extraction of debug information
+		oe_runmake CONFIG_MODULE_SIG_ALL=n DEPMOD=echo MODLIB=${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${D}${nonarch_base_libdir}/firmware modules_install
 		rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/build"
 		rm -f "${D}${nonarch_base_libdir}/modules/${KERNEL_VERSION}/source"
 		# Remove empty module directories to prevent QA issues
@@ -497,6 +498,16 @@  kernel_do_install() {
 	! [ -e Module.symvers ] || install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
 }
 
+sign_kernel_modules() {
+	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
+	if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
+		# modinst_pre= prevents the cleaning of the target before signing
+		oe_runmake -C ${B} modinst_pre= DEPMOD=echo MODLIB=${PKGD}${nonarch_base_libdir}/modules/${KERNEL_VERSION} INSTALL_FW_PATH=${PKGD}${nonarch_base_libdir}/firmware modules_sign
+	fi
+}
+
+PACKAGEBUILDPKGD += "sign_kernel_modules"
+
 # Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
 do_kernel_version_sanity_check() {
 	if [ "x${KERNEL_VERSION_SANITY_SKIP}" = "x1" ]; then