[PATCH][SRU][E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][SRU][E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink

Seth Forshee
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 <[hidden email]>
---
 debian/rules.d/2-binary-arch.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
index 82e4d80e469f..2aea5e857f79 100644
--- a/debian/rules.d/2-binary-arch.mk
+++ b/debian/rules.d/2-binary-arch.mk
@@ -413,10 +413,14 @@ 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 \
+ while IFS= read -r -d '' signature < <(tail -c 28 "$$path_module"); do \
+ break; \
+ done; \
  $(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 && \
+   [ "$$signature" = $$'~Module signature appended~\n' ]; then \
  $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
  $(MODSECKEY) \
  $(MODPUBKEY) \
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][SRU][E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink

Juerg Haefliger
On Tue, 29 Oct 2019 08:15:25 -0500
Seth Forshee <[hidden email]> 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 <[hidden email]>
> ---
>  debian/rules.d/2-binary-arch.mk | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
> index 82e4d80e469f..2aea5e857f79 100644
> --- a/debian/rules.d/2-binary-arch.mk
> +++ b/debian/rules.d/2-binary-arch.mk
> @@ -413,10 +413,14 @@ 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 \
> + while IFS= read -r -d '' signature < <(tail -c 28 "$$path_module"); do \
> + break; \
> + done; \
I'm not a big fan of this since I don't understand why the while..do is
necessary. Also, can't we simply do 'signature=$(tail -c 28 "$$path_module")?
Yes, that drops the trailing newline but imho if we get the '~Module signature
appended~' string back then we can be fairly certain that the module is signed.

Or just let modinfo do the parsing then we don't have to read a magic number of
bytes from the end of the module:
signer=$(modinfo -F signer '$$path_module'). If the string is non-empty, the
module is signed.

>   $(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 && \
> +   [ "$$signature" = $$'~Module signature appended~\n' ]; then \

and then
[ -n "$$signer" ]; then \

>   $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
>   $(MODSECKEY) \
>   $(MODPUBKEY) \

...Juerg

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

attachment0 (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

NAK: [PATCH][SRU][E/Unstable] UBUNTU: [Packaging] Leave unsigned modules unsigned after adding .gnu_debuglink

Seth Forshee
On Tue, Oct 29, 2019 at 05:10:25PM +0100, Juerg Haefliger wrote:

> On Tue, 29 Oct 2019 08:15:25 -0500
> Seth Forshee <[hidden email]> 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 <[hidden email]>
> > ---
> >  debian/rules.d/2-binary-arch.mk | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
> > index 82e4d80e469f..2aea5e857f79 100644
> > --- a/debian/rules.d/2-binary-arch.mk
> > +++ b/debian/rules.d/2-binary-arch.mk
> > @@ -413,10 +413,14 @@ 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 \
> > + while IFS= read -r -d '' signature < <(tail -c 28 "$$path_module"); do \
> > + break; \
> > + done; \
>
> I'm not a big fan of this since I don't understand why the while..do is
> necessary. Also, can't we simply do 'signature=$(tail -c 28 "$$path_module")?
> Yes, that drops the trailing newline but imho if we get the '~Module signature
> appended~' string back then we can be fairly certain that the module is signed.
>
> Or just let modinfo do the parsing then we don't have to read a magic number of
> bytes from the end of the module:
> signer=$(modinfo -F signer '$$path_module'). If the string is non-empty, the
> module is signed.
>
> >   $(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 && \
> > +   [ "$$signature" = $$'~Module signature appended~\n' ]; then \
>
> and then
> [ -n "$$signer" ]; then \

Didn't know that modinfo supported this. Based on  looking at the
source, this is a better check anyhow, so I'll update the patch to use
that.

Thanks,
Seth

>
> >   $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \
> >   $(MODSECKEY) \
> >   $(MODPUBKEY) \
>
> ...Juerg



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team