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

fuzz: simplify fuzzers dependencies in CIFuzz #1896

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

IvanNardi
Copy link
Collaborator

@IvanNardi IvanNardi commented Mar 2, 2023

CIFuzz (based on oss-fuzz) is the GitHub action/CI job that runs fuzz
targets on pull requests. It only runs the fuzzers affected by a pull
request/commit. Otherwise it will divide up the allotted fuzzing time
among all fuzzers in the project.
Since:

  • we have more than 20 fuzzers and most of them use the custom memory
    allocation functions (to force allocation failures) even if they are not
    strictly about DPI stuff;
  • we need to keep fuzzing time relatively small (to avoid waiting for the CI
    results for a long time)

it is important that fuzzers dependencies (which are based on files
changed by the single commit/PR) are as small as possible.

Bottom line: move all the low-level allocation callbacks to a dedicated
file; this way most of the fuzzers don't depend anymore on ndpi_main.c
file (which is touched by every commit/PR).

The goal is to have only the "most important" fuzzers running during (most
of) the CI.

@IvanNardi IvanNardi marked this pull request as draft March 2, 2023 14:34
@IvanNardi IvanNardi marked this pull request as ready for review March 2, 2023 14:58
Copy link
Collaborator

@utoni utoni left a comment

Choose a reason for hiding this comment

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

I do not know what the intention of those flow memory wrappers were in the first place. But in my imagination, it could be useful e.g. for fixed size memory pools.

I am not sure if this feature is used by developers. But I think it has it's justification.

@IvanNardi
Copy link
Collaborator Author

IvanNardi commented Mar 13, 2023

I do not know what the intention of those flow memory wrappers were in the first place. But in my imagination, it could be useful e.g. for fixed size memory pools.

I am not sure if this feature is used by developers. But I think it has it's justification.

Ok, I'll push a new version restoring the flow wrappers

CIFuzz (based on oss-fuzz) is the GitHub action/CI job that runs fuzz
targets on pull requests. It only runs the fuzzers affected by a pull
request/commit. Otherwise it will divide up the allotted fuzzing time
among all fuzzers in the project.
Since:
* we have more than 20 fuzzers and most of them use the custom memory
allocation functions (to force allocation failures) even if they are not
strictly about DPI stuff;
* we need to keep fuzzing time relatively small (to avoid waiting the CI
results for a long time)

it is important that fuzzers dependencies (which are based on *files*
changed by the single commit/PR) are as small as possible.

Bottom line: move all the low-level allocation callbacks to a dedicated
file; this way most of the fuzzers don't depend anymore on `ndpi_main.c`
file (which is touched by ever commit/PR).

The goal is to have only the "most important" fuzzers running during (most
of) the CI.
@sonarcloud
Copy link

sonarcloud bot commented Mar 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@IvanNardi IvanNardi merged commit 9eff075 into ntop:dev Mar 14, 2023
@IvanNardi IvanNardi deleted the cifuzz-dependencies branch March 14, 2023 18:34
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.

2 participants