evilsocket / opensnitch

OpenSnitch is a GNU/Linux interactive application firewall inspired by Little Snitch.
GNU General Public License v3.0
10.9k stars 509 forks source link

several race conditions in eBPF program #426

Open evilsocket opened 3 years ago

evilsocket commented 3 years ago

I believe that increments like this one should be done via:

__sync_fetch_and_add(value, 1)

according to this:

Since the defined array map is global, the accounting needs to use an atomic operation, which is defined as lock_xadd(). LLVM maps __sync_fetch_and_add() as a built-in function to the BPF atomic add instruction, that is, BPF_STX | BPF_XADD | BPF_W for word sizes.

i'm not an eBPF expert so before moving forward with the fixes i'd like to hear from @gustavo-iniguez-goya and @themighty1

themighty1 commented 3 years ago

Hi, @evilsocket , I think you are right, it might be racy. However, before making any changes, one need to make sure that bpf in older kernels like 4.4 supports this atomic instruction.

evilsocket commented 3 years ago

i'm not even sure that the precompiled eBPF program would run on anything but 5.x ( see #427 ) ... anyways, that primitive is translated to

BPF_STX | BPF_XADD | BPF_W

and i believe these opcodes are there since eBPF was there ... not sure

gustavo-iniguez-goya commented 3 years ago

tested on 4.12.14, 4.15.0, 4.18, 5.4.0, 5.8.4 and 5.10.x (and 4.19 (i386)). On 4.9 (debian9) fails with error while loading kprobe/tcp_v4_connect

evilsocket commented 3 years ago

@gustavo-iniguez-goya what's the grep BPF /boot/config-$(uname -r) for that debian?

gustavo-iniguez-goya commented 3 years ago

CONFIG_BPF=y CONFIG_BPF_SYSCALL=y CONFIG_BPF_JIT=y CONFIG_BPF_EVENTS=y

CONFIG_KPROBES=y CONFIG_KPROBE_EVENT=y

gustavo-iniguez-goya commented 3 years ago

On the other hand, I've realized that we fail to enable this method on Debian Buster, because for some reason IPv6 established sockets can not be dumped via netlink:

eBPF could not dump TCPv6 sockets via netlink: Warning, no message nor error from netlink
eBPF error in dumping TCPv6 sockets via netlink

we fail here: https://github.com/evilsocket/opensnitch/blob/master/daemon/procmon/ebpf/ebpf.go#L140 . Letting it continue without returning there, ebpf works as expected.

Some systems/users disable IPv6, and I added a check (IPv6Enabled = Exists("/proc/sys/net/ipv6")) to verify this scenario, but on this system that directory is populated, so there must be something else going on there.

But maybe the problem is that there're no ipv6 connections... no idea, I'll investigate it.

gustavo-iniguez-goya commented 3 years ago

But maybe the problem is that there're no ipv6 connections... no idea, I'll investigate it.

ok, that's the problem, no TCP IPv6 connections, so we can ignore that error and keep working if eBPF doesn't fail to load.

e5b54f0