Re: [PATCH 02/15] scripts/sbom: integrate script in make process

[Luis Augenstein <luis.augenstein@tngtech.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 4107 bytes --]

Hi everyone,

> If sbom.py is unable to parse the build commands, does it exit with a
> non-zero exit code, correct?

yes correct. The current behavior is to always parse as much as
possible, collect all problems, print an error summary in the end and
exit with a non-zero exit code.

>> The cmd macro uses 'set -e', so consider moving this up and making it
>>
>>     trap  "rm -rf $$roots_file" EXIT; \
>>
>> like try-run in scripts/Makefile.compiler does to ensure it is always
>> cleaned up.
>
> hm, well.  Yes, this should do as expected, but please be aware that
> this also kills the $(delete-on-interrupt) which is part of $(cmd) and
> removes $@ in case of error or interruption by installing a trap --
> which will be overwritten.  See also below.

I think $(delete-on-interrupt) only operates on non-phony targets. Since
our target is $@=sbom instead of the generated .spdx.json files
$(delete-on-interrupt) currently has no effect and wouldn't really be
"killed by intention" by introducing a trap within cmd_sbom.

> there should be _no_ output on error, regardless of
--write-output-on-error.

If this is a general convention we should follow, then we maybe want to
return a zero exit code in sbom.py in case of errors when
--write-output-on-error is set, effectively treating errors as warnings?
Alternatively, we could keep not using --write-output-on-error by
default. Or we don't follow the convention and write the output
*.spdx.json files in case of errors.

I am not sure what's the most appropriate behavior here. However, we
know that people will very likely encounter cases with commands unknown
to the parser. It's not very useful to simply deny generating the entire
documents, because in many cases the sbom will still be quite usable
although less complete. For example, if the parsing error occurs close
to the leaf nodes it cuts off only a small part of the dependency graph
which still retains most of the information.
Tim Bird apparently already encountered this problem which lead him to
manually add the --write-output-on-error flag.
https://birdcloud.org/bc/Linux_Kernel_Missing_SPDX_ID_lines_from_build_SBOMs


> The cmd macro uses 'set -e', so consider moving this up and making it
>
>     trap  "rm -rf $$roots_file" EXIT; \
>
> like try-run in scripts/Makefile.compiler does to ensure it is always
> cleaned up.
and
> The common way in kbuild is using '$(tmp-target)'.

This would be the new version then:

# Script to generate .spdx.json SBOM documents describing the build
#
---------------------------------------------------------------------------

ifdef building_out_of_srctree
sbom_targets := sbom-source.spdx.json
endif
sbom_targets += sbom-build.spdx.json sbom-output.spdx.json
quiet_cmd_sbom = GEN     $(notdir $(sbom_targets))
      cmd_sbom = roots_file="$(tmp-target)"; \
	             trap "rm -rf $$roots_file" EXIT; \
                 printf "%s\n" "$(KBUILD_IMAGE)" >"$$roots_file"; \
                 $(if $(CONFIG_MODULES),sed 's/\.o$$/.ko/'
$(objtree)/modules.order >> "$$roots_file";) \
                 $(PYTHON3) $(srctree)/scripts/sbom/sbom.py \
                     --src-tree $(abspath $(srctree)) \
                     --obj-tree $(abspath $(objtree)) \
                     --roots-file "$$roots_file" \
                     --output-directory $(abspath $(objtree)) \
                     --generate-spdx \
                     --package-license "GPL-2.0 WITH Linux-syscall-note" \
                     --package-version "$(KERNELVERSION)" \
                     --write-output-on-error;
PHONY += sbom
sbom: $(notdir $(KBUILD_IMAGE)) include/generated/autoconf.h $(if
$(CONFIG_MODULES),modules modules.order)
	$(call cmd,sbom)

thanks,
Luis

-- 
Luis Augenstein * luis.augenstein@tngtech.com * +49-152-25275761
TNG Technology Consulting GmbH, Beta-Str. 13, 85774 Unterföhring
Geschäftsführer: Henrik Klagges, Dr. Robert Dahlke, Thomas Endres
Aufsichtsratsvorsitzender: Christoph Stock
Sitz: Unterföhring * Amtsgericht München * HRB 135082

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3207 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]