Project

General

Profile

Actions

Bug #2654

closed

Off-by-one iteration of EBPF flow_table_vX in EBPFForEachFlowVXTable (util-ebpf.c)

Added by Gatewatcher Dev Team about 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
low
Difficulty:
low
Label:

Description

Hi,

We believe we discovered a bug in util-ebpf.c, function
EBPFForEachFlowV4Table, relative to the usage of the
bpf_map_get_next_key "syscall". The same bug can be found in
EBPFForEachFlowV6Table 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 the
bpf_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

Actions #1

Updated by Victor Julien almost 6 years ago

  • Target version changed from 4.1 to 4.1.1
Actions #2

Updated by Eric Leblond almost 6 years ago

  • Assignee set to Eric Leblond
Actions #3

Updated by Victor Julien almost 6 years ago

  • Target version changed from 4.1.1 to 70
Actions #4

Updated by Eric Leblond over 5 years ago

  • Status changed from New to Resolved
Actions #5

Updated by Eric Leblond over 5 years ago

  • Status changed from Resolved to Closed
Actions #7

Updated by Eric Leblond over 5 years ago

  • Target version changed from 70 to 5.0rc1
Actions

Also available in: Atom PDF