Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't add ATC table name to registry until after sqlite DB initialization #8233

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

astrocox
Copy link
Contributor

@astrocox astrocox commented Jan 3, 2024

Fixes #8232

I'm not sure if there are nuances I'm missing or if there is a good way to test this section of code, but this fixes a bug where the first ATC table specified in a conf file is attached twice and the second attempt fails.

@astrocox astrocox requested review from a team as code owners January 3, 2024 22:20
Copy link

linux-foundation-easycla bot commented Jan 3, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: astrocox / name: Julia Cox (c0a28d2)

@directionless directionless added this to the 5.12.0 milestone Jan 7, 2024
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this in office hours today. The conclusion is that no one understands why this bug is present, but this fix seems okay.

@directionless directionless merged commit c5145f0 into osquery:master Feb 27, 2024
16 checks passed
@astrocox
Copy link
Contributor Author

We discussed this in office hours today. The conclusion is that no one understands why this bug is present, but this fix seems okay.

Thanks for reviewing/merging! Sorry I missed the office hours agenda. If anyone is curious, this bug was happening because Registry::call() calls getConnection() on the SQLliteDBInstance, which calls attachVirtualTables() if the db has not previously been initialized. The attachVirtualTables() function then iterates over every table in the registry and calls Registry::call() for each one. So if you add to the list of tables before that first DB initialization happens, that first table will have attachTableInternal() called on it twice.

Bad screenshot of the offending stack trace:
Screenshot 2024-01-03 154356

@directionless
Copy link
Member

Thank you so much for contributing the patch!

@rotadsr
Copy link

rotadsr commented Mar 1, 2024

I was having the same issue for moz_bookmarks:

E0301 11:34:47.283597 -640033728 virtual_table.cpp:1115] Error creating named virtual table: firefox_bookmarks (1)

And I was guessing this could be due to the path in the ATC file:

{
  "auto_table_construction": {
    "firefox_bookmarks": {
      "platform": "darwin",
      "query": "SELECT * FROM moz_bookmarks;",
      "path": "/Users/%/Library/Application Support/Firefox/Profiles/%/places.sqlite",
      "columns": [
        "id",
        "type",
        "fk",
        "parent",
        "position",
        "title",
        "keyword_id",
        "folder_type",
        "dateAdded",
        "lastModified",
        "guid",
        "syncStatus",
        "syncChangeCounter"
      ]
    }
}

There are two (or probably more) profiles in the Profiles folder, so here comes the problem. I guess the fix will also be solving this, right?

Thanks!

@directionless
Copy link
Member

Heads up, this may have broken something around ATC table creation via plugins. We're still debugging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Error creating named virtual table" with ATC because osquery tries to create the table twice
3 participants