Re: [PATCH v12 4/5] security: keys: trusted: use ASN.1 TPM2 key format for the blobs

[Nick Desaulniers <ndesaulniers@google.com>]

On Mon, Sep 21, 2020 at 2:31 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Mon, 2020-09-21 at 08:07 +0800, kernel test robot wrote:
> > Hi James,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on integrity/next-integrity]
> > [also build test ERROR on linus/master v5.9-rc5 next-20200918]
> > [cannot apply to security/next-testing dhowells-fs/fscache-next]
> > [If your patch is applied to the wrong git tree, kindly drop us a
> > note. And when submitting patch, we suggest to use '--base' as
> > documented in https://git-scm.com/docs/git-format-patch]
> >
> > url:
> > https://github.com/0day-ci/linux/commits/James-Bottomley/TPM-2-0-trusted-key-rework/20200921-003922
> > base:
> > https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> > next-integrity
> > config: x86_64-randconfig-a003-20200921 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
> > f4e554180962aa6bc93678898b6933ea712bde50)
> > reproduce (this is a W=1 build):
> >         wget
> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install x86_64 cross compiling tool for clang build
> >         # apt-get install binutils-x86-64-linux-gnu
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> > ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> > > > make[4]: *** No rule to make target 'security/keys/trusted-
> > > > keys/tpm2key.asn1.o', needed by 'security/keys/trusted-
> > > > keys/built-in.a'.
> >    make[4]: *** [scripts/Makefile.build:283: security/keys/trusted-
> > keys/trusted_tpm2.o] Error 1
> >    make[4]: Target '__build' not remade because of errors.
>
>
> So can I still add that tracking this down involved installing an
> entirely unnecessary ARM build environment, which was a huge effort I
> didn't need to do if you'd just provided the build log which fingered
> the ASN.1 compiler problem if you know what to look for.

Having a link to the build log artifact is a valid criticism.

>
> The reason for the problem is because ASN1 isn't selected in the
> Kconfig which causes the ASN.1 compiler not to be built.  The way our
> current build rules are structured causes the make rule for this simply
> to be skipped, which means you have to know to look for the absence of
> ASN.1 in the build log.  I propose adding this to the build rules,
> which produces the much more explicit:
>
> /home/jejb/git/linux-build/scripts/Makefile.build:387: *** CONFIG_ASN1 must be defined for the asn1_compiler.  Stop.
> make[3]: *** [/home/jejb/git/linux-build/scripts/Makefile.build:505: security/keys/trusted-keys] Error 2
>
> James
>
> ---
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index a467b9323442..bca7003beac8 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -382,6 +382,11 @@ quiet_cmd_asn1_compiler = ASN.1   $(basename $@).[ch]
>        cmd_asn1_compiler = $(objtree)/scripts/asn1_compiler $< \
>                                 $(basename $@).c $(basename $@).h
>
> +ifndef CONFIG_ASN1
> +$(objtree)/scripts/asn1_compiler:
> +       $(error CONFIG_ASN1 must be defined for the asn1_compiler)
> +endif
> +
>  $(obj)/%.asn1.c $(obj)/%.asn1.h: $(src)/%.asn1 $(objtree)/scripts/asn1_compiler
>         $(call cmd,asn1_compiler)

Is there a better way via Kconfig to gate whatever consumes
CONFIG_ASN1 on CONFIG_ASN1 being set, rather than erroring for
randconfig builds? I don't see how the diff would solve the case of CI
systems doing randconfig builds.
-- 
Thanks,
~Nick Desaulniers