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

Coverity fixes v3 #7169

Closed
wants to merge 4 commits into from
Closed

Conversation

catenacyber
Copy link
Contributor

@catenacyber catenacyber commented Mar 24, 2022

Link to redmine ticket:
None, not sure it deserves one

Describe changes: patches to make coverity happy

  • Prevents a double unlock (by locking again)
  • Makes static code analyzer about a memory leak in xbits parsing (removing dead code for default switch cases)
  • Asserts all cases are covered in debuglog (PKT_IS_IPV6 or PKT_IS_IPV4)
  • fix possible null dereference in debug validation in PacketPoolDestroy

After this and #7139 (which I think is a real bug this one), there will be no more outstanding issues reported by coverity

Replaces #7138 with review taken into account

Even if the code seems unreachable for now
Coverity ID: 1502953

As we check just on the next line my_pool against NULL, we should
not dereference it, even for debug validation
@catenacyber catenacyber requested a review from a team as a code owner March 24, 2022 20:54
@catenacyber catenacyber mentioned this pull request Mar 24, 2022
@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 6637

/* since fb lock is still held this flow won't be found until we are done */
FLOWLOCK_UNLOCK(old_f);
// if f->use_cnt == 0, flow will be unlocked by caller with MoveToWorkQueue
if (f->use_cnt > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

does this even compile? Should be old_f?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

@catenacyber catenacyber mentioned this pull request Mar 25, 2022
@catenacyber
Copy link
Contributor Author

Replaced by #7173

jasonish added a commit to jasonish/suricata that referenced this pull request Feb 11, 2025
By default, use an empty search path. This gives us a predictable
default. If a user needs access to external modules, the search path
must be set in the configuration file.

Ticket: OISF#7169
jasonish added a commit to jasonish/suricata that referenced this pull request Feb 12, 2025
By default, use an empty search path. This gives us a predictable
default. If a user needs access to external modules, the search path
must be set in the configuration file.

Ticket: OISF#7169
jasonish added a commit to jasonish/suricata that referenced this pull request Feb 12, 2025
By default, use an empty search path. This gives us a predictable
default. If a user needs access to external modules, the search path
must be set in the configuration file.

Ticket: OISF#7169
jasonish added a commit to jasonish/suricata that referenced this pull request Feb 13, 2025
By default, use an empty search path. This gives us a predictable
default. If a user needs access to external modules, the search path
must be set in the configuration file.

Ticket: OISF#7169
victorjulien pushed a commit to victorjulien/suricata that referenced this pull request Feb 14, 2025
By default, use an empty search path. This gives us a predictable
default. If a user needs access to external modules, the search path
must be set in the configuration file.

Ticket: OISF#7169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants