| Message ID | AM9PR09MB46421C5E3A51479C951310FDA83B9@AM9PR09MB4642.eurprd09.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] cve-check: use local copy instead of global db | expand |
On Tue, Feb 22, 2022 at 07:46:59PM +0100, Konrad Weihmann wrote: >instead of using a global DB (default in DL_DIR) copy an existing >DB file into WORKDIR and use it from there. >This should avoid running into the reported "database is readonly" error >while at the same time there's no need to arbritrarily limit the >cve_check task to just one run at a time I tested this on dunfell branch (the patch needed some minor context tweaks in order to apply). The slowdown I observed during do_cve_check seems to be gone, and the actual CVE report remains unchanged. Will keep testing it some more, but for now, it seems an improvement. Tested-by: Ralph Siemsen <ralph.siemsen@linaro.org>
This seems really suboptimal. The only operation which writes to the sqlite database is the fetch task of cve-update-db-native, everything else is read-only. Or should be... I just discovered a function which was failing to open the database read-only, which is possible the cause of the problem. Patches on the list, this improves "bitbake core-image-sato --runall cve_check" from nearly 4 minutes to 25 seconds on my machine, with 64 bitbake jobs in parallel. Ross On Tue, 22 Feb 2022 at 18:47, Konrad Weihmann <kweihmann@outlook.com> wrote: > > instead of using a global DB (default in DL_DIR) copy an existing > DB file into WORKDIR and use it from there. > This should avoid running into the reported "database is readonly" error > while at the same time there's no need to arbritrarily limit the > cve_check task to just one run at a time > > Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> > --- > This patch should be tested by users that run cve-check on a regular > on hosts with as much as possible cores, before merging. > In local testing I haven't found any issues on a world build, > but as mentioned in the previous patch the issue is kind of hard to > reproduce. > So this patch aims at lifting the arbitrary task lock, while preventing > access by more than one thread/process at a time by sqlite. > Feedback through heavy local testing is very much appreciated > > meta/classes/cve-check.bbclass | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass > index 21d3da7974..e4389b7001 100644 > --- a/meta/classes/cve-check.bbclass > +++ b/meta/classes/cve-check.bbclass > @@ -28,6 +28,7 @@ CVE_VERSION ??= "${PV}" > CVE_CHECK_DB_DIR ?= "${DL_DIR}/CVE_CHECK" > CVE_CHECK_DB_FILE ?= "${CVE_CHECK_DB_DIR}/nvdcve_1.1.db" > CVE_CHECK_DB_FILE_LOCK ?= "${CVE_CHECK_DB_FILE}.lock" > +CVE_CHECK_DB_FILE_LOCAL = "${WORKDIR}/${@os.path.basename(d.getVar('CVE_CHECK_DB_FILE'))}" > > CVE_CHECK_LOG ?= "${T}/cve.log" > CVE_CHECK_TMP_FILE ?= "${TMPDIR}/cve_check" > @@ -94,9 +95,11 @@ python do_cve_check () { > """ > Check recipe for patched and unpatched CVEs > """ > + import bb.utils > from oe.cve_check import get_patched_cves > > if os.path.exists(d.getVar("CVE_CHECK_DB_FILE")): > + bb.utils.copyfile(d.getVar("CVE_CHECK_DB_FILE"), d.getVar("CVE_CHECK_DB_FILE_LOCAL")) > try: > patched_cves = get_patched_cves(d) > except FileNotFoundError: > @@ -111,7 +114,6 @@ python do_cve_check () { > } > > addtask cve_check before do_build after do_fetch > -do_cve_check[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}" > do_cve_check[depends] = "cve-update-db-native:do_fetch" > do_cve_check[nostamp] = "1" > > @@ -185,7 +187,7 @@ def check_cves(d, patched_cves): > cve_whitelist = d.getVar("CVE_CHECK_WHITELIST").split() > > import sqlite3 > - db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro") > + db_file = d.expand("file:${CVE_CHECK_DB_FILE_LOCAL}?mode=ro") > conn = sqlite3.connect(db_file, uri=True) > > # For each of the known product names (e.g. curl has CPEs using curl and libcurl)... > -- > 2.25.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#162190): https://lists.openembedded.org/g/openembedded-core/message/162190 > Mute This Topic: https://lists.openembedded.org/mt/89323890/1676615 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ross@burtonini.com] > -=-=-=-=-=-=-=-=-=-=-=- >
diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass index 21d3da7974..e4389b7001 100644 --- a/meta/classes/cve-check.bbclass +++ b/meta/classes/cve-check.bbclass @@ -28,6 +28,7 @@ CVE_VERSION ??= "${PV}" CVE_CHECK_DB_DIR ?= "${DL_DIR}/CVE_CHECK" CVE_CHECK_DB_FILE ?= "${CVE_CHECK_DB_DIR}/nvdcve_1.1.db" CVE_CHECK_DB_FILE_LOCK ?= "${CVE_CHECK_DB_FILE}.lock" +CVE_CHECK_DB_FILE_LOCAL = "${WORKDIR}/${@os.path.basename(d.getVar('CVE_CHECK_DB_FILE'))}" CVE_CHECK_LOG ?= "${T}/cve.log" CVE_CHECK_TMP_FILE ?= "${TMPDIR}/cve_check" @@ -94,9 +95,11 @@ python do_cve_check () { """ Check recipe for patched and unpatched CVEs """ + import bb.utils from oe.cve_check import get_patched_cves if os.path.exists(d.getVar("CVE_CHECK_DB_FILE")): + bb.utils.copyfile(d.getVar("CVE_CHECK_DB_FILE"), d.getVar("CVE_CHECK_DB_FILE_LOCAL")) try: patched_cves = get_patched_cves(d) except FileNotFoundError: @@ -111,7 +114,6 @@ python do_cve_check () { } addtask cve_check before do_build after do_fetch -do_cve_check[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}" do_cve_check[depends] = "cve-update-db-native:do_fetch" do_cve_check[nostamp] = "1" @@ -185,7 +187,7 @@ def check_cves(d, patched_cves): cve_whitelist = d.getVar("CVE_CHECK_WHITELIST").split() import sqlite3 - db_file = d.expand("file:${CVE_CHECK_DB_FILE}?mode=ro") + db_file = d.expand("file:${CVE_CHECK_DB_FILE_LOCAL}?mode=ro") conn = sqlite3.connect(db_file, uri=True) # For each of the known product names (e.g. curl has CPEs using curl and libcurl)...
instead of using a global DB (default in DL_DIR) copy an existing DB file into WORKDIR and use it from there. This should avoid running into the reported "database is readonly" error while at the same time there's no need to arbritrarily limit the cve_check task to just one run at a time Signed-off-by: Konrad Weihmann <kweihmann@outlook.com> --- This patch should be tested by users that run cve-check on a regular on hosts with as much as possible cores, before merging. In local testing I haven't found any issues on a world build, but as mentioned in the previous patch the issue is kind of hard to reproduce. So this patch aims at lifting the arbitrary task lock, while preventing access by more than one thread/process at a time by sqlite. Feedback through heavy local testing is very much appreciated meta/classes/cve-check.bbclass | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)