| Message ID | 20190314175705.11671-1-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: If the module stripping command fails the build should abort | expand |
On Fri, Mar 15, 2019 at 2:59 AM Douglas Anderson <dianders@chromium.org> wrote: > > If the call to strip returns an error code then it makes sense for the > build to fail. Currently we'll just chug along and ship unstripped > modules. > > Fixes: e2a666d52b48 ("kbuild: sign the modules at install time") > Signed-off-by: Douglas Anderson <dianders@chromium.org> Did you see this problem in the latest kernel? Since commit 3a2429e1faf40b2aaa481aa4b001a74d222c7e8b, $(call cmd,...) is run with 'set -e'. Any failure in a series of commands will let the build fail. If you have the problem in old versions ( < 4.20), I do not mind this patch for linux-stable. > --- > > scripts/Makefile.modinst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > index ff5ca9817a85..5d05c43c6f31 100644 > --- a/scripts/Makefile.modinst > +++ b/scripts/Makefile.modinst > @@ -22,7 +22,7 @@ quiet_cmd_modules_install = INSTALL $@ > cmd_modules_install = \ > mkdir -p $(2) ; \ > cp $@ $(2) ; \ > - $(mod_strip_cmd) $(2)/$(notdir $@) ; \ > + $(mod_strip_cmd) $(2)/$(notdir $@) && \ > $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) && \ > $(mod_compress_cmd) $(2)/$(notdir $@) > > -- > 2.21.0.360.g471c308f928-goog > -- Best Regards Masahiro Yamada
Hi, On Fri, Mar 15, 2019 at 7:11 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Fri, Mar 15, 2019 at 2:59 AM Douglas Anderson <dianders@chromium.org> wrote: > > > > If the call to strip returns an error code then it makes sense for the > > build to fail. Currently we'll just chug along and ship unstripped > > modules. > > > > Fixes: e2a666d52b48 ("kbuild: sign the modules at install time") > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > Did you see this problem in the latest kernel? > > Since commit q, > $(call cmd,...) is run with 'set -e'. > > Any failure in a series of commands will let the build fail. > > > If you have the problem in old versions ( < 4.20), Ah! I was in 4.19 when I saw the problem. I then confirmed that the code in mainline was the same and that the new version built fine with my patch, but I didn't go back and confirm the problem there. OK, I just checked linux/master and can confirm there's no problem there. Sorry for the noise then... I wonder if perhaps we should revert commit caf6fe91ddf6 ("modsign: Abort modules_install when signing fails") then to be consistent? > I do not mind this patch for linux-stable. It's probably not worth it. In general I prefer linux-stable to be as just cherry-picks of mainline as much as possible. When it starts forking then future picks get harder. Sure in this case it's unlikely that someone will get tripped up by an "&&" vs a ";" when picking future changes, but given that it's not super urgent I guess I'd vote that we skip it. -Doug -Doug
Hi. On Sat, Mar 16, 2019 at 12:19 AM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Fri, Mar 15, 2019 at 7:11 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > On Fri, Mar 15, 2019 at 2:59 AM Douglas Anderson <dianders@chromium.org> wrote: > > > > > > If the call to strip returns an error code then it makes sense for the > > > build to fail. Currently we'll just chug along and ship unstripped > > > modules. > > > > > > Fixes: e2a666d52b48 ("kbuild: sign the modules at install time") > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > > > Did you see this problem in the latest kernel? > > > > Since commit q, > > $(call cmd,...) is run with 'set -e'. > > > > Any failure in a series of commands will let the build fail. > > > > > > If you have the problem in old versions ( < 4.20), > > Ah! I was in 4.19 when I saw the problem. I then confirmed that the > code in mainline was the same and that the new version built fine with > my patch, but I didn't go back and confirm the problem there. > > OK, I just checked linux/master and can confirm there's no problem > there. Sorry for the noise then... > > I wonder if perhaps we should revert commit caf6fe91ddf6 ("modsign: > Abort modules_install when signing fails") then to be consistent? Right. If you send a patch, I will take it. > > I do not mind this patch for linux-stable. > > It's probably not worth it. In general I prefer linux-stable to be as > just cherry-picks of mainline as much as possible. When it starts > forking then future picks get harder. Sure in this case it's unlikely > that someone will get tripped up by an "&&" vs a ";" when picking > future changes, but given that it's not super urgent I guess I'd vote > that we skip it. Agree.
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst index ff5ca9817a85..5d05c43c6f31 100644 --- a/scripts/Makefile.modinst +++ b/scripts/Makefile.modinst @@ -22,7 +22,7 @@ quiet_cmd_modules_install = INSTALL $@ cmd_modules_install = \ mkdir -p $(2) ; \ cp $@ $(2) ; \ - $(mod_strip_cmd) $(2)/$(notdir $@) ; \ + $(mod_strip_cmd) $(2)/$(notdir $@) && \ $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) && \ $(mod_compress_cmd) $(2)/$(notdir $@)
If the call to strip returns an error code then it makes sense for the build to fail. Currently we'll just chug along and ship unstripped modules. Fixes: e2a666d52b48 ("kbuild: sign the modules at install time") Signed-off-by: Douglas Anderson <dianders@chromium.org> --- scripts/Makefile.modinst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)