Skip to content

Conversation

@siteshwar
Copy link
Member

@gregkh
Copy link

gregkh commented Apr 30, 2025

What?

No, you shouldn't add foolish files for broken tools that no one should be using. That's just noise, fix the tools instead to NOT say stuff like this!

Again, no, this is NOT how you do any of this at all.

@kdudka
Copy link
Member

kdudka commented Apr 30, 2025

@gregkh The findings were reported by open-source static analyzers. We are not developers of the tools. We only automate the scanning. The tools were not chosen randomly though. There are thousands of success stories behind them, both internally at Red Hat and in upstream projects. If you find other open-source static analyzers more useful than what OSH uses in the default scanning configuration, we are open to suggestions.

@gregkh
Copy link

gregkh commented Apr 30, 2025

Don't "automate" using tools that are broken, that feels like a failing proposition in the long run as who will then go back and review all of these to verify if they have changed or not?

And did you verify that all of these issues are really false-positives? Where is the documentation that shows that the tool is wrong here, and why not submit that information to the tool authors so that it can be fixed instead of just papering over all of this?

@kdudka
Copy link
Member

kdudka commented Apr 30, 2025

Good point. We should append a comment to each finding to explain why we think it is a false positive before this get merged, like we did it for bash: https://github.com/openscanhub/known-false-positives/blob/5eea38e13238e0ca25be9fb3c31300bda38fb92f/bash/ignore.err

Cppcheck is not based on a real C compiler, so its precision is limited by design. Still the tool was able to find true positives for example in Linux kernel: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=40251b8eb46e48c011939a3ddf056fe13a223319

GCC Analyzer is still under heavy development and many bugs are being reported and fixed in the tool. We are in touch with the main developer of the tool but a deep analysis of each single false positive is not possible with the amount of data we get in a Fedora mass scan.

@gregkh
Copy link

gregkh commented Apr 30, 2025

We are in touch with the main developer of the tool but a deep analysis of each single false positive is not possible with the amount of data we get in a Fedora mass scan.

Then perhaps you shouldn't be doing mass-scans and then sending them out to others to have them review them all like you all did? Please be much more mindful of maintainers/developers time.

@siteshwar
Copy link
Member Author

Then perhaps you shouldn't be doing mass-scans and then sending them out to others to have them review them all like you all did? Please be much more mindful of maintainers/developers time.

While running mass scans on Fedora, the reports should be verified by the package maintainer or someone from the community to make the reports more useful for the upstream developers. There would always be false positives in reports from static analysis tools that should be either filtered out or the analyzers should be fixed not to report them. Thanks for your feedback!

Related: gregkh/usbutils#229

Signed-off-by: Siteshwar Vashisht <svashisht@redhat.com>
@kdudka kdudka self-requested a review April 30, 2025 15:38
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

I checked the justifications for the recorded false positives and they look valid to me.

@siteshwar siteshwar merged commit ced2b93 into openscanhub:main Apr 30, 2025
kdudka added a commit to kdudka/csdiff that referenced this pull request Apr 30, 2025
... so that we do not end up with paths like this:
```
usbutils-018/redhat-linux-build/../usbmisc.c
```

Related: openscanhub/known-false-positives#6
kdudka added a commit to kdudka/csdiff that referenced this pull request May 1, 2025
... so that we do not end up with paths like this:
```
usbutils-018/redhat-linux-build/../usbmisc.c
```

Related: openscanhub/known-false-positives#6
Related: openscanhub/known-false-positives#7
PR: csutils#228
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.

3 participants