Hello Antonin,
Thank you for the review.
On Monday, March 23, 2026 at 9:28 AM, Antonin Godard wrote:
> Hi,
>
> Some suggestions and questions I picked while reviewing this patch.
>
> On Thu Mar 19, 2026 at 4:42 PM CET, Benjamin Robin wrote:
> > By default, the sbom-cve-check class generates these export files:
> > - A JSON in `cve-check` format, named `${IMAGE_NAME}.cve-check.json`
> > - An SPDX 3.0 SBOM, named `${IMAGE_NAME}.cve-check.spdx.json`.
> >
> > A user can add or remove export file formats by using the
> > `SBOM_CVE_CHECK_EXPORT_VARS` variable.
> >
> > By default, the CVE databases are downloaded using the following
> > recipes:
> > - sbom-cve-check-update-cvelist-native.bb
> > - sbom-cve-check-update-nvd-native.bb
> >
> > The database fetch and deploy logic is implemented in
> > sbom-cve-check-update-db.inc. The CVE databases are stored in the
> > download directory (`DL_DIR`) by default. This can be configured by
> > the `SBOM_CVE_CHECK_DATABASES_DIR` variable defined in
> > meta/recipes-core/meta/sbom-cve-check-config.inc.
>
> Yet the default definition is:
>
> SBOM_CVE_CHECK_DATABASES_DIR ??= "${DEPLOY_DIR}/sbom_cve_check/databases"
>
> Which does not point to the DL_DIR? The commit message seems a bit misleading.
In the v7 this is going to be fixed.
> > diff --git a/meta/classes-recipe/sbom-cve-check.bbclass
> > b/meta/classes-recipe/sbom-cve-check.bbclass
> > new file mode 100644
> > index 000000000000..32f92a0bab29
> > --- /dev/null
> > +++ b/meta/classes-recipe/sbom-cve-check.bbclass
> > @@ -0,0 +1,121 @@
> > +# SPDX-License-Identifier: MIT
> > +
> > +# To enable this class, it is recommended to add this to local.conf
> > +# OE_FRAGMENTS += "core/yocto/sbom-cve-check"
>
> I'd rather recommend the tool normally used to manage fragments, which
> modifies
> toolcfg.conf, made for this purpose.
>
> ```
> # It is recommended to enable this class through the sbom-cve-check fragment:
> # bitbake-config-build enable-fragment sbom-cve-check
> ```
I improved the comment, but I kept the original explanation.
For example this could be useful for writing KAS configuration.
> > +
> > +require recipes-core/meta/sbom-cve-check-config.inc
> > +
> > +SBOM_CVE_CHECK_DEPLOYDIR = "${WORKDIR}/sbom_cve_check/image-deploy"
>
> Nitpick, but why use underscore for "sbom_cve_check"? Here, and in the other
> places in this patch. Using "sbom-cve-check" everywhere would perhaps be more
> coherent?
I standardized all directory names, it should be named with dash now.
> > +SBOM_CVE_CHECK_EXTRA_ARGS[doc] = "Allow to specify extra arguments to
> > sbom-cve-check. \
> > + For example to add export flags for filtering (e.g., only export
> > vulnerable CVEs). \
> > +"
> > +SBOM_CVE_CHECK_EXTRA_ARGS ??= ""
> > +
> > +SBOM_CVE_CHECK_EXPORT_VARS[doc] = "List of variables that declare export
> > files to generate. \
> > + Each variable must have a 'type' and an 'ext' flag set. \
> > + The 'type' flag contains the value that is passed to the --export-type
> > command flags. \
> > + The 'ext' flag contains the filename extension (suffix). The output
> > filename is going \
> > + to be ${IMAGE_NAME}${ext} \
> > +"
> > +SBOM_CVE_CHECK_EXPORT_VARS ?= "SBOM_CVE_CHECK_EXPORT_SPDX3
> > SBOM_CVE_CHECK_EXPORT_CVECHECK"
> > +
> > +SBOM_CVE_CHECK_EXPORT_SPDX3[doc] = "Export configuration to generate an
> > SPDX3 SBOM file, \
> > + with the following name: ${IMAGE_NAME}.cve-check.spdx.json \
> > +"
> > +SBOM_CVE_CHECK_EXPORT_SPDX3[type] ?= "spdx3"
> > +SBOM_CVE_CHECK_EXPORT_SPDX3[ext] ?= ".cve-check.spdx.json"
> > +
> > +SBOM_CVE_CHECK_EXPORT_CVECHECK[doc] = "Export configuration to generate a
> > JSON manifest \
> > + in the same format as the cve-check class, with the following name: \
> > + ${IMAGE_NAME}.cve-check.json \
> > +"
> > +SBOM_CVE_CHECK_EXPORT_CVECHECK[type] ?= "yocto-cve-check-manifest"
> > +SBOM_CVE_CHECK_EXPORT_CVECHECK[ext] ?= ".cve-check.json"
>
> As I understand it, these represent the different kinds of outputs of
> sbom-cve-check. Wouldn't these make more sense as:
>
> SBOM_CVE_CHECK_EXPORT_SPDX3[ext] ?= ".sbom-cve-check.spdx.json"
> SBOM_CVE_CHECK_EXPORT_CVECHECK[ext] ?= ".sbom-cve-check.yocto.json"
>
> first item indicates that this is an sbom-cve-check generated file,
> second item denotes the format of the file : either spdx3, or the yocto
> specific
> one.
Yeah you are write, it is clearer this way, despite the fact that is it
a bit longer.
> > diff --git a/meta/conf/fragments/yocto/sbom-cve-check.conf
> > b/meta/conf/fragments/yocto/sbom-cve-check.conf
> > new file mode 100644
> > index 000000000000..a3f229acf28a
> > --- /dev/null
> > +++ b/meta/conf/fragments/yocto/sbom-cve-check.conf
> > @@ -0,0 +1,14 @@
> > +BB_CONF_FRAGMENT_SUMMARY = "This fragment enables sbom-cve-check with
> > recommended default options"
> > +BB_CONF_FRAGMENT_DESCRIPTION = "Enables sbom-cve-check and applies the
> > following configurations: \
> > + - Adds the sbom-cve-check class to IMAGE_CLASSES. \
> > + - Configures CVE database recipes to fetch the latest git revision using
> > AUTOREV. \
> > + - Ensures generated SBOM includes all CVE annotations. \
> > + - Configures the Linux kernel recipe to provide compiled sources, \
> > + allowing CVEs to be excluded if the source is not compiled. \
> > +"
> > +
> > +IMAGE_CLASSES:append = " sbom-cve-check"
> > +SRCREV:pn-sbom-cve-check-update-nvd-native = "${AUTOREV}"
> > +SRCREV:pn-sbom-cve-check-update-cvelist-native = "${AUTOREV}"
> > +SPDX_INCLUDE_VEX = "all"
> > +SPDX_INCLUDE_COMPILED_SOURCES:pn-linux-yocto = "1"
>
> Could we also set SBOM_CVE_CHECK_ALLOW_NETWORK to 1 here? Since we already
> require network with AUTOREV anyway.
The do_sbom_cve_check task does not need network by default.
It is only needed if the user want to fetch additional databases, for
example annotation databases.
> > diff --git a/meta/recipes-core/meta/sbom-cve-check-config.inc
> > b/meta/recipes-core/meta/sbom-cve-check-config.inc
> > new file mode 100644
> > index 000000000000..a1a909e22250
> > --- /dev/null
> > +++ b/meta/recipes-core/meta/sbom-cve-check-config.inc
>
> This file and others could be in a more appropriate directory, such are
> recipes-devtools/sbom-cve-check/? I think in a patch review call we said that
> this directory was wrongly used for the previous cve-check-related recipes
> (you
> weren't in this call IIRC).
This is going to be fixed in the v7 series: The recipes are moved to
recipes-devtools/sbom-cve-check/.
> > @@ -0,0 +1,4 @@
> > +# SPDX-License-Identifier: MIT
> > +
> > +SBOM_CVE_CHECK_DATABASES_DIR ??= "${DEPLOY_DIR}/sbom_cve_check/databases"
>
> I think there is already "${DEPLOY_DIR}/cve" used by cve-check and vex, maybe
> you could re-use this path as:
>
> SBOM_CVE_CHECK_DATABASES_DIR ??= "${DEPLOY_DIR}/cve/sbom_cve_check/databases"
>
> to avoid creating a new directory in DEPLOY_DIR?
I really think this is a bad idea. The ${DEPLOY_DIR}/cve/ directory contains
a lot of json file at the root of this directory. I really do not want to
break existing VEX and/or cve-check tasks.
> > +SBOM_CVE_CHECK_DATABASES_DIR[doc] = "Download directory path where to
> > store the CVE databases"
> > diff --git a/meta/recipes-core/meta/sbom-cve-check-update-cvelist-native.bb
> > b/meta/recipes-core/meta/sbom-cve-check-update-cvelist-native.bb
> > new file mode 100644
> > index 000000000000..ce204db6c51a
> > --- /dev/null
> > +++ b/meta/recipes-core/meta/sbom-cve-check-update-cvelist-native.bb
> > @@ -0,0 +1,12 @@
> > +SUMMARY = "Updates the CVE List database"
> > +LICENSE = "MIT"
> > +LIC_FILES_CHKSUM =
> > "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda2f7b4f302"
> > +
> > +HOMEPAGE = "https://github.com/CVEProject/cvelistV5";
> > +SRC_URI =
> > "git://github.com/CVEProject/cvelistV5.git;branch=main;protocol=https"
> > +SBOM_CVE_CHECK_DB_NAME = "cvelist"
> > +
> > +# 2026-03-19_baseline
> > +SRCREV = "ada54ee3cc8380820aa45e4996910bdc9dcb94e7"
> > +
> > +require sbom-cve-check-update-db.inc
> > diff --git a/meta/recipes-core/meta/sbom-cve-check-update-db.inc
> > b/meta/recipes-core/meta/sbom-cve-check-update-db.inc
> > new file mode 100644
> > index 000000000000..5ecb79820247
> > --- /dev/null
> > +++ b/meta/recipes-core/meta/sbom-cve-check-update-db.inc
> > @@ -0,0 +1,28 @@
> > +# SPDX-License-Identifier: MIT
> > +
> > +INHIBIT_DEFAULT_DEPS = "1"
> > +EXCLUDE_FROM_WORLD = "1"
> > +
> > +inherit native
> > +require sbom-cve-check-config.inc
> > +
> > +SBOM_CVE_CHECK_DB_NAME[doc] = "Database name, which is the Git repository
> > directory name. \
> > + The git repository will be stored in ${SBOM_CVE_CHECK_DATABASES_DIR)/"
> > +
> > +DEPENDS += "rsync-native"
> > +
> > +# Leverage BitBake's checksum computation for populated sysroot files to
> > determine
> > +# whether other recipe tasks dependent on this output need to be
> > re-executed.
> > +do_compile() {
> > + git -C "${S}" rev-parse --verify "HEAD^{object}" >
> > "${WORKDIR}/${SBOM_CVE_CHECK_DB_NAME}.rev"
>
> We're in do_compile(), so you can store the output file in ${B}?
>
> git -C "${S}" rev-parse --verify "HEAD^{object}" >
> "${B}/${SBOM_CVE_CHECK_DB_NAME}.rev"
This part is going to be removed since it is no longer necessary.
> > +}
> > +
> > +# In the install task, also deploy directly to ${DEPLOY_DIR} using rsync.
> > +# This is an hack, we are not using do_deploy to prevent multiple
> > unecessary copy of the CVE database.
> > +do_install() {
> > + install -m 644 -D -t "${D}${datadir}/sbom_cve_check/databases/"
> > "${WORKDIR}/${SBOM_CVE_CHECK_DB_NAME}.rev"
> > +
> > + dst="${SBOM_CVE_CHECK_DATABASES_DIR}/${SBOM_CVE_CHECK_DB_NAME}"
> > + mkdir -p "$dst"
> > + rsync -aH --delete --link-dest="${S}/" "${S}/" "${dst}/"
> > +}
>
> Why do we need this in both ${datadir} and the deploy dir? Isn't
> sbom-cve-check only using the one in the deploy dir? Was this done to create a
> package out of these recipes? If we don't really need one we could use
> ALLOW_EMPTY:${PN} = "1" instead?
Indeed, we could do that, and in this case the do_sbom_cve_check task should
depend on the do_install task. I am going to use ALLOW_EMPTY on this v7
series.
> > diff --git a/meta/recipes-core/meta/sbom-cve-check-update-nvd-native.bb
> > b/meta/recipes-core/meta/sbom-cve-check-update-nvd-native.bb
> > new file mode 100644
> > index 000000000000..46c86952a164
> > --- /dev/null
> > +++ b/meta/recipes-core/meta/sbom-cve-check-update-nvd-native.bb
> > @@ -0,0 +1,12 @@
> > +SUMMARY = "Updates the NVD CVE database"
> > +LICENSE = "cve-tou"
> > +LIC_FILES_CHKSUM =
> > "file://LICENSES/cve-tou.md;md5=bc5bbf146f01e20ece63d83c8916d8fb"
> > +
> > +HOMEPAGE = "https://github.com/fkie-cad/nvd-json-data-feeds";
> > +SRC_URI =
> > "git://github.com/fkie-cad/nvd-json-data-feeds.git;branch=main;protocol=https"
> > +SBOM_CVE_CHECK_DB_NAME = "nvd-fkie"
> > +
> > +# v2026.03.19-010002
> > +SRCREV = "49f8bbe1b0b0884e16bdc37ab68db997085570a7"
> > +
> > +require sbom-cve-check-update-db.inc
Thanks,
--
Benjamin Robin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#233710):
https://lists.openembedded.org/g/openembedded-core/message/233710
Mute This Topic: https://lists.openembedded.org/mt/118402422/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
