Bug #2654
closedOff-by-one iteration of EBPF flow_table_vX in EBPFForEachFlowVXTable (util-ebpf.c)
Description
Hi,
We believe we discovered a bug in util-ebpf.c, functionEBPFForEachFlowV4Table
, relative to the usage of thebpf_map_get_next_key
"syscall". The same bug can be found inEBPFForEachFlowV6Table
since the functions are basically duplicates.
This bug causes an off-by-one walk of the BPF flow_table_v4
/flow_table_v6
maps with the counters from the very last entry not being accounted.
This causes the coalesced flowstats counters to be inaccurate.
The bug origin comes from the incorrect handling of thebpf_map_get_next_key()
return value:
while (bpf_map_get_next_key(mapfd, &key, &next_key) == 0) {
According to documentation (bpf(2)), section BPF_MAP_GET_NEXT_KEY:
If key is found, the operation returns zero and sets the
next_key pointer to the key of the next element. If key is
not found, the operation returns zero and sets the next_key
pointer to the key of the first element. If key is the last
element, -1 is returned and errno is set to ENOENT.
This means that the very last bpf_map_get_next_key() call will return
-1, but this does not mean that the current key does not exist. It only
means that it is the last one. As such, we ought to do one last loop
cycle to handle the current key and then exit the loop.
We suggest changing the while expression to the evaluation of a new
"boolean", set by default to true. This "boolean" would be changed to
false at the top of the while block when bpf_map_get_next_key
would
return -1 with errno
equal to ENOENT
.
Cheers,
ArmatureTech Dev Team
Updated by Victor Julien about 6 years ago
- Target version changed from 4.1 to 4.1.1
Updated by Victor Julien almost 6 years ago
- Target version changed from 4.1.1 to 70
Updated by Eric Leblond over 5 years ago
Fixed in PR: https://github.com/OISF/suricata/pull/3948
Updated by Eric Leblond over 5 years ago
- Target version changed from 70 to 5.0rc1