a022648b9e
The Suricata AF_PACKET code opens a socket per thread, then after some minor setup enters a loop where the socket is poll()'d with a timeout. When the poll() call returns a non zero positive value, the AF_PACKET ring will be processed. The ringbuffer processing logic has a pointer into the ring where we last checked the ring. From this position we will inspect each frame until we find a frame with tp_status == TP_STATUS_KERNEL (so essentially 0). This means the frame is currently owned by the kernel. There is a special case handling for starting the ring processing but finding a TP_STATUS_KERNEL immediately. This logic then skip to the next frame, rerun the check, etc until it either finds an initialized frame or the last frame of the ringbuffer. The problem was, however, that the initial uninitialized frame was possibly (likely?) still being initialized by the kernel. A data race between the notification through the socket (the poll()) and the updating of the `tp_status` field in the frame could lead to a valid frame getting skipped. Of note is that for example libpcap does not do frame scanning. Instead it simply exits it ring processing loop. Also interesting is that libpcap uses atomic loads and stores on the tp_status field. This skipping of frames had 2 bad side effects: 1. in most cases, the buffer would be full enough that the frame would be processed in the next pass of the ring, but now the frame would out of order. This might have lead to packets belong to the same flow getting processed in the wrong order. 2. more severe is the soft lockup case. The skipped frame sits at ring buffer index 0. The rest of the ring has been cleared, after the initial frame was skipped. As our pass of the ring stops at the end of the ring (ptv->frame_offset + 1 == ptv->req.v2.tp_frame_nr) the code exits the ring processing loop at goes back to poll(). However, poll() will not indicate that there is more data, as the stale frame in the ring blocks the kernel from populating more frames beyond it. This is now a dead lock, as the kernel waits for Suricata and Suricata never touches the ring until it hears from the kernel. The scan logic will scan the whole ring at most once, so it won't reconsider the stale frame either. This patch addresses the issues in several ways: 1. the startup "discard" logic was fixed to not skip over kernel frames. Doing so would get us in a bad state at start up. 2. Instead of scanning the ring, we now enter a busy wait loop when encountering a kernel frame where we didn't expect one. This means that if we got a > 0 poll() result, we'll busy wait until we get at least one frame. 3. Error handling is unified and cleaned up. Any frame error now returns the frame to the kernel and progresses the frame pointer. 4. If we find a frame that is owned by us (TP_STATUS_USER_BUSY) we yield to poll() immediately, as the next expected status of that frame is TP_STATUS_KERNEL. 5. the ring is no longer processed until the "end" of the ring (so highest index), but instead we process at most one full ring size per run. 6. Work with a copy of `tp_status` instead of accessing original touched also by the kernel. Bug: #4785. |
5 years ago | |
|---|---|---|
| .github | 5 years ago | |
| benches | ||
| contrib | ||
| doc | 5 years ago | |
| ebpf | 5 years ago | |
| etc | ||
| lua | ||
| python | ||
| qa | 5 years ago | |
| rules | 5 years ago | |
| rust | 5 years ago | |
| scripts | 5 years ago | |
| src | 5 years ago | |
| suricata-update | ||
| .clang-format | ||
| .gitignore | 5 years ago | |
| .lgtm.yml | 5 years ago | |
| .readthedocs.yaml | 5 years ago | |
| COPYING | ||
| ChangeLog | ||
| LICENSE | ||
| Makefile.am | 5 years ago | |
| Makefile.cvs | ||
| README.md | ||
| acsite.m4 | ||
| autogen.sh | ||
| config.rpath | ||
| configure.ac | 5 years ago | |
| doxygen.cfg | 5 years ago | |
| libsuricata-config.in | ||
| requirements.txt | 5 years ago | |
| suricata.yaml.in | 5 years ago | |
| threshold.config | ||
README.md
Suricata
Introduction
Suricata is a network IDS, IPS and NSM engine.
Installation
https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Suricata_Installation
User Guide
You can follow the Suricata user guide to get started.
Our deprecated (but still useful) user guide is also available.
Contributing
We're happily taking patches and other contributions. Please see https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Contributing for how to get started.
Suricata is a complex piece of software dealing with mostly untrusted input. Mishandling this input will have serious consequences:
- in IPS mode a crash may knock a network offline;
- in passive mode a compromise of the IDS may lead to loss of critical and confidential data;
- missed detection may lead to undetected compromise of the network.
In other words, we think the stakes are pretty high, especially since in many common cases the IDS/IPS will be directly reachable by an attacker.
For this reason, we have developed a QA process that is quite extensive. A consequence is that contributing to Suricata can be a somewhat lengthy process.
On a high level, the steps are:
-
Travis-CI based build & unit testing. This runs automatically when a pull request is made.
-
Review by devs from the team and community
-
QA runs
Overview of Suricata's QA steps
Trusted devs and core team members are able to submit builds to our (semi) public Buildbot instance. It will run a series of build tests and a regression suite to confirm no existing features break.
The final QA run takes a few hours minimally, and is started by Victor. It currently runs:
- extensive build tests on different OS', compilers, optimization levels, configure features
- static code analysis using cppcheck, scan-build
- runtime code analysis using valgrind, DrMemory, AddressSanitizer, LeakSanitizer
- regression tests for past bugs
- output validation of logging
- unix socket testing
- pcap based fuzz testing using ASAN and LSAN
Next to these tests, based on the type of code change further tests can be run manually:
- traffic replay testing (multi-gigabit)
- large pcap collection processing (multi-terabytes)
- fuzz testing (might take multiple days or even weeks)
- pcap based performance testing
- live performance testing
- various other manual tests based on evaluation of the proposed changes
It's important to realize that almost all of the tests above are used as acceptance tests. If something fails, it's up to you to address this in your code.
One step of the QA is currently run post-merge. We submit builds to the Coverity Scan program. Due to limitations of this (free) service, we can submit once a day max. Of course it can happen that after the merge the community will find issues. For both cases we request you to help address the issues as they may come up.
FAQ
Q: Will you accept my PR?
A: That depends on a number of things, including the code quality. With new features it also depends on whether the team and/or the community think the feature is useful, how much it affects other code and features, the risk of performance regressions, etc.
Q: When will my PR be merged?
A: It depends, if it's a major feature or considered a high risk change, it will probably go into the next major version.
Q: Why was my PR closed?
A: As documented in the Suricata Github workflow here https://redmine.openinfosecfoundation.org/projects/suricata/wiki/Github_work_flow, we expect a new pull request for every change.
Normally, the team (or community) will give feedback on a pull request after which it is expected to be replaced by an improved PR. So look at the comments. If you disagree with the comments we can still discuss them in the closed PR.
If the PR was closed without comments it's likely due to QA failure. If the Travis-CI check failed, the PR should be fixed right away. No need for a discussion about it, unless you believe the QA failure is incorrect.
Q: the compiler/code analyser/tool is wrong, what now?
A: To assist in the automation of the QA, we're not accepting warnings or errors to stay. In some cases this could mean that we add a suppression if the tool supports that (e.g. valgrind, DrMemory). Some warnings can be disabled. In some exceptional cases the only 'solution' is to refactor the code to work around a static code checker limitation false positive. While frustrating, we prefer this over leaving warnings in the output. Warnings tend to get ignored and then increase risk of hiding other warnings.
Q: I think your QA test is wrong
A: If you really think it is, we can discuss how to improve it. But don't come to this conclusion too quickly, more often it's the code that turns out to be wrong.
Q: do you require signing of a contributor license agreement?
A: Yes, we do this to keep the ownership of Suricata in one hand: the Open Information Security Foundation. See http://suricata-ids.org/about/open-source/ and http://suricata-ids.org/about/contribution-agreement/