[v2,SRU,E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink
diff mbox series

Message ID 20191029192513.21447-1-seth.forshee@canonical.com
State New
Headers show
Series
  • [v2,SRU,E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink
Related show

Commit Message

Seth Forshee Oct. 29, 2019, 7:25 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1850234

When adding .gnu_debuglink sections to modules we sign modules
without regard to whether or not they were signed previously. As
a result modules from staging which should not have been signed
are ending up with signature. Change this to check for a module
signature before modifying the binary, then sign the result only
if the original module was signed.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 debian/rules.d/2-binary-arch.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Khaled Elmously Oct. 29, 2019, 8:25 p.m. UTC | #1
On 2019-10-29 14:25:13 , Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1850234
> 
> When adding .gnu_debuglink sections to modules we sign modules
> without regard to whether or not they were signed previously. As
> a result modules from staging which should not have been signed
> are ending up with signature. Change this to check for a module
> signature before modifying the binary, then sign the result only
> if the original module was signed.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  debian/rules.d/2-binary-arch.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
> index 82e4d80e469f..050f867060cb 100644
> --- a/debian/rules.d/2-binary-arch.mk
> +++ b/debian/rules.d/2-binary-arch.mk
> @@ -413,10 +413,12 @@ ifneq ($(skipdbg),true)
>  	  -name '*.ko' | while read path_module ; do \
>  		module="/lib/modules/$${path_module#*/lib/modules/}"; \
>  		if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \
> +			signer=$$(/sbin/modinfo -F signer "$$path_module"); \
>  			$(CROSS_COMPILE)objcopy \
>  				--add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \
>  				$$path_module; \
> -			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config; then \
> +			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \
> +			   [ -n "$$signer" ]; then \
>  				$(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
>  					$(MODSECKEY) \
>  					$(MODPUBKEY) \

Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
Sultan Alsawaf Oct. 29, 2019, 8:34 p.m. UTC | #2
Acked-by: Sultan Alsawaf <sultan.alsawaf@canonical.com>

On Tue, Oct 29, 2019, 12:25 PM Seth Forshee <seth.forshee@canonical.com>
wrote:

> BugLink: https://bugs.launchpad.net/bugs/1850234
>
> When adding .gnu_debuglink sections to modules we sign modules
> without regard to whether or not they were signed previously. As
> a result modules from staging which should not have been signed
> are ending up with signature. Change this to check for a module
> signature before modifying the binary, then sign the result only
> if the original module was signed.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  debian/rules.d/2-binary-arch.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/
> 2-binary-arch.mk
> index 82e4d80e469f..050f867060cb 100644
> --- a/debian/rules.d/2-binary-arch.mk
> +++ b/debian/rules.d/2-binary-arch.mk
> @@ -413,10 +413,12 @@ ifneq ($(skipdbg),true)
>           -name '*.ko' | while read path_module ; do \
>                 module="/lib/modules/$${path_module#*/lib/modules/}"; \
>                 if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \
> +                       signer=$$(/sbin/modinfo -F signer
> "$$path_module"); \
>                         $(CROSS_COMPILE)objcopy \
>
> --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \
>                                 $$path_module; \
> -                       if grep -q CONFIG_MODULE_SIG=y
> $(builddir)/build-$*/.config; then \
> +                       if grep -q CONFIG_MODULE_SIG=y
> $(builddir)/build-$*/.config && \
> +                          [ -n "$$signer" ]; then \
>                                 $(builddir)/build-$*/scripts/sign-file
> $(MODHASHALGO) \
>                                         $(MODSECKEY) \
>                                         $(MODPUBKEY) \
> --
> 2.20.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Seth Forshee Oct. 29, 2019, 8:48 p.m. UTC | #3
On Tue, Oct 29, 2019 at 02:25:13PM -0500, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1850234
> 
> When adding .gnu_debuglink sections to modules we sign modules
> without regard to whether or not they were signed previously. As
> a result modules from staging which should not have been signed
> are ending up with signature. Change this to check for a module
> signature before modifying the binary, then sign the result only
> if the original module was signed.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Applied to unstable/master.
Stefan Bader Nov. 7, 2019, 4:06 p.m. UTC | #4
On 29.10.19 20:25, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1850234
> 
> When adding .gnu_debuglink sections to modules we sign modules
> without regard to whether or not they were signed previously. As
> a result modules from staging which should not have been signed
> are ending up with signature. Change this to check for a module
> signature before modifying the binary, then sign the result only
> if the original module was signed.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---

This was already applied to Eoan in a re-spin. Sending message for the records.

-Stefan
>  debian/rules.d/2-binary-arch.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
> index 82e4d80e469f..050f867060cb 100644
> --- a/debian/rules.d/2-binary-arch.mk
> +++ b/debian/rules.d/2-binary-arch.mk
> @@ -413,10 +413,12 @@ ifneq ($(skipdbg),true)
>  	  -name '*.ko' | while read path_module ; do \
>  		module="/lib/modules/$${path_module#*/lib/modules/}"; \
>  		if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \
> +			signer=$$(/sbin/modinfo -F signer "$$path_module"); \
>  			$(CROSS_COMPILE)objcopy \
>  				--add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \
>  				$$path_module; \
> -			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config; then \
> +			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \
> +			   [ -n "$$signer" ]; then \
>  				$(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
>  					$(MODSECKEY) \
>  					$(MODPUBKEY) \
>

Patch
diff mbox series

diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
index 82e4d80e469f..050f867060cb 100644
--- a/debian/rules.d/2-binary-arch.mk
+++ b/debian/rules.d/2-binary-arch.mk
@@ -413,10 +413,12 @@  ifneq ($(skipdbg),true)
 	  -name '*.ko' | while read path_module ; do \
 		module="/lib/modules/$${path_module#*/lib/modules/}"; \
 		if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \
+			signer=$$(/sbin/modinfo -F signer "$$path_module"); \
 			$(CROSS_COMPILE)objcopy \
 				--add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \
 				$$path_module; \
-			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config; then \
+			if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \
+			   [ -n "$$signer" ]; then \
 				$(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
 					$(MODSECKEY) \
 					$(MODPUBKEY) \