Bug #3846
closedInfinite loop if the sniffing interface temporarily goes down
Description
How to reproduce: start suricata in live-pcap mode. Bring the sniffing interface down with ip link set {interface_name} down
, then bring it up again.
Suricata doesn't exit, but it no longer produces events either.
The problem appears to be that function ReceivePcapLoop keeps calling PcapTryReopen in a loop, which in turn keeps returning PCAP_ERROR_ACTIVATED, resulting in an infinite loop.
The following patch fixes it:
--- a/src/source-pcap.c +++ b/src/source-pcap.c @@ -250,6 +250,7 @@ TmEcode ReceivePcapLoop(ThreadVars *tv, void *data, void *slo t) ptv->slot = s->slot_next; ptv->cb_result = TM_ECODE_OK; + TmEcode retval = TM_ECODE_OK; while (1) { if (suricata_ctl_flags & SURICATA_STOP) { @@ -278,7 +279,16 @@ TmEcode ReceivePcapLoop(ThreadVars *tv, void *data, void *sl ot) break; } r = PcapTryReopen(ptv); - } while (r < 0); + switch(r) { + case PCAP_ERROR_ACTIVATED: + /* "the operation can't be performed on already + activated captures" - permanent error, exit + */ + dbreak = 1; + retval = TM_ECODE_FAILED; + break; + } + } while (r < 0 && dbreak == 0); if (dbreak) { break; } @@ -292,7 +302,7 @@ TmEcode ReceivePcapLoop(ThreadVars *tv, void *data, void *slot) PcapDumpCounters(ptv); StatsSyncCountersIfSignalled(tv); - SCReturnInt(TM_ECODE_OK); + SCReturnInt(retval); } /**
Updated by Roland Fischer over 4 years ago
The proposed patch is one way of fixing it but it'll exit Suricata.
The question is whether Suricata should try to recover in this use case, or if it should exit and leave it up to some external process control, e.g. systemd if you fancy that, to restart the whole thing. Each solution has its tos and fros.
From what I can tell, retry behaviour was added in https://github.com/OISF/suricata/commit/fb36c0af126883f91f7bd45ad162323a9efaf031 quite a bit ago with the goal of "Prior to this patch, a suricata listening to an interface was leaving when the interface goes down. This patch modifies the behaviour to automatically reconnect.".
This probably points more towards fixing up PcapTryReopen()
to handle this use case as well, but the suricata devs would better know their preference.
Updated by Roland Fischer over 4 years ago
Fixing PcapTryReopen()
would probably require to pcap_close()
the pcap handle in this case and re-create it similar to what is done in ReceivePcapThreadInit()
.
Updated by Philippe Antoine over 3 years ago
- Status changed from New to In Review
- Target version set to 7.0.0-beta1
Updated by Juliana Fajardini Reichow over 2 years ago
- Status changed from In Review to In Progress
- Assignee set to Juliana Fajardini Reichow
This work had been started already on GH but the contributor, unfortunately, hasn't signed the CLA, so we can't accept their contribution.
Claiming it, so this bug can be resolved.
Updated by Juliana Fajardini Reichow over 2 years ago
- Status changed from In Progress to In Review
New PR for review: https://github.com/OISF/suricata/pull/7580
Updated by Juliana Fajardini Reichow over 2 years ago
- Status changed from In Review to Closed
Merged PR: https://github.com/OISF/suricata/pull/7580
Updated by Juliana Fajardini Reichow over 2 years ago
- Label Needs backport to 6.0 added
Updated by Juliana Fajardini Reichow over 2 years ago
- Copied to Bug #5436: Infinite loop if the sniffing interface temporarily goes down (6.0.x backports) added