Project

General

Profile

Actions

Bug #5882

closed

Issue with Hyperscan 5.4.1

Added by Dylan Hooker about 1 year ago. Updated 12 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
-
Affected Versions:
Effort:
Difficulty:
Label:

Description

Compiled suri 6.0.10 today on RHEL8 using a fresh git of the hyperscan library (v5.4.1 was released last week). All compilation looked fine, but starting suricata fails with <Error> - [ERRCODE: SC_ERR_FATAL(171)] - Hyperscan returned error -1. Also recompiled on suri 6.0.8 and had same issue.

Purged all HS v5.4.1 bits, recompiled previous HS v5.4.0 git, recompiled suri 6.0.10 using HS 5.4.0, and no more error.

Have not yet had a chance to dig deeper/get coredumps for further troubleshooting. Was able to consistently reproduce using 5.4.1 library on multiple hosts.

Actions #1

Updated by Dylan Hooker about 1 year ago

  • Description updated (diff)
Actions #2

Updated by Victor Julien about 1 year ago

Did Suricata log any more info?

Actions #3

Updated by Andreas Herz about 1 year ago

I can reproduce this on ArchLinux as well. Even with `-vvvv` a user would only see the mentioned error message from hyperscan. With the amount of commits we might use a git bisect to find out where they broke it and report it upstream.

Actions #4

Updated by Andreas Herz about 1 year ago

We were able to narrow it down that the commit https://github.com/intel/hyperscan/commit/277fc400892ba57ad5d9eda9f5bcbc6cc6a1b8ca is the one that introduced the issue. The one before works okay so far. @Dylan Hooker if you would like feel free to confirm that this commit breaks it for you as well and that the commit before https://github.com/intel/hyperscan/commit/5aa4bd565fb1eef6c0ef1b04f6c823e6503bd5b4 still works.

Actions #5

Updated by Dylan Hooker about 1 year ago

Yep, can confirm:
compiling with https://github.com/intel/hyperscan/commit/277fc400892ba57ad5d9eda9f5bcbc6cc6a1b8ca exhibits the crash on startup
compiling with https://github.com/intel/hyperscan/commit/5aa4bd565fb1eef6c0ef1b04f6c823e6503bd5b4 does not exhibit the crash on startup

It bombs out just after capture threads finish starting. This is the only error logged in verbose mode.
2/3/2023 -- 15:16:23 - <Info> - All AFP capture threads are running.
2/3/2023 -- 15:16:23 - <Error> - [ERRCODE: SC_ERR_FATAL(171)] - Hyperscan returned error -1

Actions #6

Updated by Victor Julien about 1 year ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Jeff Lucovsky
  • Target version changed from TBD to 7.0.0-rc2

@Jeff Lucovsky can you check this out? I haven't looked deep, but I suspect the way Suricata uses its hyperscan scratch space is no longer supported. We use a single global one, with per thread clones. It seems the new code expects a unique scratch per pattern database, and we have many such databases.

I guess the order of things should be:
- analyze the issue
- report to hyperscan to see if Suricata is correct (the current behavior has been implemented by the previous hyperscan team, so I'm kind of surprised if suri does it wrong here)
- depending on their answer fix
- if suri needs fixing, we need a backport label here

Actions #7

Updated by Victor Julien about 1 year ago

  • Priority changed from Normal to High
Actions #8

Updated by Jeff Lucovsky about 1 year ago

I'm investigating ....

The addition of the crc consistency check in master validScratch when called from hs_scan causes the issue.

     /* add quick rose sanity checks by db crc*/
     if (s->db_crc != crc) {
         DEBUG_PRINTF("Improper scratch for current db\n");
         return 0;
     }

The Hyperscan documentation [https://intel.github.io/hyperscan/dev-reference/performance.html?highlight=scratch] recommends each context have it's own scratch space for performance:

A scratch space can be allocated so that it can be used with any one of a number of databases. Each concurrent scan operation (such as a thread) needs its own scratch space.

I manually added the same check to 5aa4bd565fb1eef6c0ef1b04f6c823e6503bd5b4 and removed the early return statement. There were no detected inconsistencies between db_crc and crc

With that check removed, Suricata's unittests and suricata-verify tests execute successfully.

Actions #11

Updated by Jeff Lucovsky about 1 year ago

  • Status changed from Assigned to In Progress
Actions #12

Updated by Victor Julien 12 months ago

  • Status changed from In Progress to Closed
  • Priority changed from High to Normal
  • Target version deleted (7.0.0-rc2)
Actions

Also available in: Atom PDF