diff mbox series

cve-update-nvd2-native: Tweak to work better with NFS DL_DIR

Message ID 20241218115634.1666697-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit f642edb006a8c16dbe45681afe547eabfae17073
Headers show
Series cve-update-nvd2-native: Tweak to work better with NFS DL_DIR | expand

Commit Message

Richard Purdie Dec. 18, 2024, 11:56 a.m. UTC
After much debugging, the corruption issues on the autobuilder appear to
be due to the way sqlite accesses database files. It doesn't change the
file timestamp after making changes, which for reasons unknown, confuses
NFS. As soon as the file is touched, NFS becomes fine again accross the
whole cluster, as if by magic.

We could try and debug further but putting a "touch" call into the code
is easy and harmless. Lets hope this removes this annoying source of
errors.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/recipes-core/meta/cve-update-nvd2-native.bb | 2 ++
 1 file changed, 2 insertions(+)

Comments

Marko, Peter March 1, 2025, 10:01 a.m. UTC | #1
Hi Richard,

I believe that this patch is causing problem with the database never expiring.
I was wondering why valkyrie reports for master (but not other branches)
started lagging behind reality and this may be the root-cause.
Currently it's off by 1 for kernel CVEs and it's also missing puzzles CVEs.
And this commit is the only one in addition to styhead branch.

NVD update code is checking database file timestamp when deciding if
partial or full update is required, so that code always decides for partial
update due to this file touch, rendering the logic non-working.
And we know that partial updates are inaccurate, and we need to do full update
from time to time to fix wrong/missing entries.

From this point of view this commit is wrong and should be reverted.
Or rather replaced by something different.
Maybe we could add release name to the database file instead so that
old branches with different sqlite version are not allowed to modify it?

Peter

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Richard Purdie via
> lists.openembedded.org
> Sent: Wednesday, December 18, 2024 12:57
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH] cve-update-nvd2-native: Tweak to work better with
> NFS DL_DIR
> 
> After much debugging, the corruption issues on the autobuilder appear to
> be due to the way sqlite accesses database files. It doesn't change the
> file timestamp after making changes, which for reasons unknown, confuses
> NFS. As soon as the file is touched, NFS becomes fine again accross the
> whole cluster, as if by magic.
> 
> We could try and debug further but putting a "touch" call into the code
> is easy and harmless. Lets hope this removes this annoying source of
> errors.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/recipes-core/meta/cve-update-nvd2-native.bb | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meta/recipes-core/meta/cve-update-nvd2-native.bb b/meta/recipes-
> core/meta/cve-update-nvd2-native.bb
> index a68a8bb89f1..8ef13bdde6f 100644
> --- a/meta/recipes-core/meta/cve-update-nvd2-native.bb
> +++ b/meta/recipes-core/meta/cve-update-nvd2-native.bb
> @@ -90,6 +90,8 @@ python do_fetch() {
>      if update_db_file(db_tmp_file, d, database_time) == True:
>          # Update downloaded correctly, can swap files
>          shutil.move(db_tmp_file, db_file)
> +        # Need to 'touch' the file to ensure NFS sees the data
> +        os.utime(db_file)
>      else:
>          # Update failed, do not modify the database
>          bb.warn("CVE database update failed")
Richard Purdie March 1, 2025, 10:12 a.m. UTC | #2
Hi Peter,

On Sat, 2025-03-01 at 10:01 +0000, Marko, Peter wrote:
> I believe that this patch is causing problem with the database never expiring.
> I was wondering why valkyrie reports for master (but not other branches)
> started lagging behind reality and this may be the root-cause.
> Currently it's off by 1 for kernel CVEs and it's also missing puzzles CVEs.
> And this commit is the only one in addition to styhead branch.
> 
> NVD update code is checking database file timestamp when deciding if
> partial or full update is required, so that code always decides for partial
> update due to this file touch, rendering the logic non-working.
> And we know that partial updates are inaccurate, and we need to do full update
> from time to time to fix wrong/missing entries.
> 
> From this point of view this commit is wrong and should be reverted.
> Or rather replaced by something different.
> Maybe we could add release name to the database file instead so that
> old branches with different sqlite version are not allowed to modify it?

I think we saw issues even after this change so we can probably revert
it to be honest.

That said, there are deeper issues here if that is a problem. I'd note
that the updates to the database should also ideally be fixed but I
know there are API issues in doing that.

We did try changing the location for some releases but not others and
the corruption issue persists which is suggestive of some bad
interaction with NFS at this point.

We are hacking around this server/autobuilder side right now and
ultimately I think we need to move away from the sqlite database
format/usage like this. I wish I could find help to try and fix issues
properly but everyone just wants to bandaid their particular problem
:/.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/recipes-core/meta/cve-update-nvd2-native.bb b/meta/recipes-core/meta/cve-update-nvd2-native.bb
index a68a8bb89f1..8ef13bdde6f 100644
--- a/meta/recipes-core/meta/cve-update-nvd2-native.bb
+++ b/meta/recipes-core/meta/cve-update-nvd2-native.bb
@@ -90,6 +90,8 @@  python do_fetch() {
     if update_db_file(db_tmp_file, d, database_time) == True:
         # Update downloaded correctly, can swap files
         shutil.move(db_tmp_file, db_file)
+        # Need to 'touch' the file to ensure NFS sees the data
+        os.utime(db_file)
     else:
         # Update failed, do not modify the database
         bb.warn("CVE database update failed")