KernelSymbolTable helper is conceptually broken

[rafaeldtinoco]

Background

I'm working on fixing #3653, and I can create libbpfgo methods to attach kprobe on specific offsets, with:

diff --git a/libbpfgo.c b/libbpfgo.c
index 67cd88e..ae8d87b 100644
--- a/libbpfgo.c
+++ b/libbpfgo.c
@@ -1,4 +1,5 @@
 #include "libbpfgo.h"
+#include <bpf/libbpf.h>
 
 extern void loggerCallback(enum libbpf_print_level level, char *output);
 extern void perfCallback(void *ctx, int cpu, void *data, __u32 size);
@@ -300,6 +301,30 @@ void cgo_bpf_tc_hook_free(struct bpf_tc_hook *hook)
     free(hook);
 }
 
+struct bpf_kprobe_opts *cgo_bpf_kprobe_opts_new(__u64 bpf_cookie,
+                                                size_t offset,
+                                                bool retprobe,
+                                                int attach_mode)
+{
+    struct bpf_kprobe_opts *opts;
+    opts = calloc(1, sizeof(*opts));
+    if (!opts)
+        return NULL;
+
+    opts->sz = sizeof(*opts);
+    opts->bpf_cookie = bpf_cookie;
+    opts->offset = offset;
+    opts->retprobe = retprobe;
+    opts->attach_mode = attach_mode;
+
+    return opts;
+}
+
+void cgo_bpf_kprobe_opts_free(struct bpf_kprobe_opts *opts)
+{
+    free(opts);
+}
+

and

creating methods:

// AttachKprobeOnOffset attaches the BPFProgram to entry of the symbol in the kernel (at specified offset).
func (p *BPFProg) AttachKprobeOnOffset(symbol string, offset uint64) (*BPFLink, error) {
	return doAttachKprobeOnOffset(p, symbol, offset, false)
}

// AttachKretprobeOnOffset attaches the BPFProgram to exit of the symbol in the kernel (at specified offset).
func (p *BPFProg) AttachKretprobeOnOffset(symbol string, offset uint64) (*BPFLink, error) {
	return doAttachKprobeOnOffset(p, symbol, offset, true)
}

but Tracee "lazy ksymbols" logic won't work for symbols having two addresses.

Reason

The whole "lazy" concept for the kallsyms file relies in the concept that symbols would have a single address only (and the assumption that the file is mostly sorted by symbol addresses).

Checking kernel_symbols.go and the KernelSymbolTable interface, one could change GetSymbolByName to return a slice of []*KernelSymbols and that would be an easy change for fullKernelSystemTable.

Problem is that the lazy implementation relies in stopping to read the kallsym file once a symbol is picked, or in a binary search of address considering file is sorted, etc. That doesn't work well for the same symbol having more than 1 address.

Quickly checking kallsyms for duplicate symbol addresses, the generic names will have huge amount of duplicates:

1693 __func__.0
1198 _entry.1
834 __func__.2
777 _entry.3
658 __func__.1
656 _entry.2
619 _entry.5
589 __func__.4
589 _entry.4
566 __func__.3
541 _entry.6
516 _entry.7
453 _entry.8
452 __func__.5
428 __func__.6
421 _entry.9
419 _entry.10
397 _entry.11
380 __func__.7
376 _entry.12
371 __func__.8
355 _entry.13

and, the "unique kernel symbols" will have 2 or 3 addresses (under certain circumstances, like when the symbol is static to a source file, or under certain compilation optimizations):

2 switch_mm
2 sw_fence_dummy_notify
2 suspend_attrs
2 suspend_attr_group
2 subsystem_id_show
2 str__i915__trace_system_name
2 store_rotate
2 store_energy_performance_preference
2 store
2 stop_per_cpu_kthreads
2 steal_acca
2 status_store
2 ss_files
2 src_tokens
2 src_token_list
2 sock_put
2 snd_soc_util_init
2 snapshot_fops
2 smi_buf
2 slots
2 slot_list
2 sk_lookup
2 skl_dmic_data
2 sk_drops_add.isra

There is also the case where the same address has multiple symbols:

ffffffffc0310200 b __key.22	[drm_display_helper]
ffffffffc0310200 b __key.21	[drm_display_helper]
ffffffffc0310200 b __key.22	[drm_display_helper]
ffffffffc0310200 b __key.21	[drm_display_helper]
ffffffffc0310200 b __key.20	[drm_display_helper]
ffffffffc0310200 b __key.19	[drm_display_helper]
ffffffffc0310200 b __key.18	[drm_display_helper]
ffffffffc0310200 b __key.17	[drm_display_helper]
ffffffffc0310200 b drm_dp_aux_dev_class	[drm_display_helper]

So in both cases, when indexing by sym name, or by sym address, code should account for the possibility of having multiple results.

[AlonZivony]

@rafaeldtinoco I am trying to understand.
There are multiple symbols with the same name and the same owner in the kallsyms file?
Or are there from different owners?
I guess my question in the end is - is there no way to distinguish between the different symbols with the same names?

[rafaeldtinoco]

@rafaeldtinoco I am trying to understand. There are multiple symbols with the same name and the same owner in the kallsyms file? Or are there from different owners? I guess my question in the end is - is there no way to distinguish between the different symbols with the same names?

There are multiple addresses for symbols with the same name. Symbols might, or might not, have the same owner. The examples above are all from 'system' and show the amount of addresses each symbol has.

[NDStrahilevitz]

IIRC binary searching is only done for address searching. When searching by name, the search should be linear. There's also a linear search after the binary search if it fails.
So isn't your issue related to names only? If so then only the name search needs to be fixed, most simply by changing the linear scan not to short circuit on the first symbol found (though I think you proposed a better idea offline).

[rafaeldtinoco]

So isn't your issue related to names only? If so then only the name search needs to be fixed, most simply by changing the linear scan not to short circuit on the first symbol found (though I think you proposed a better idea offline).

Both cases exist. I changed the issue description to reflect it: Multiple symbols same address, multiple addresses same symbol.

I'm just making it fast, parallelized, thread-safe and making sure it works. Feel free to optimize it further if/when needed. I don't think there should be multiple flavors as well, the unique flavor should be the optimal and used in all cases.

[rafaeldtinoco]

With aquasecurity/libbpfgo#399 merged I believe I can fix this issue in Tracee by giving the symbol offsets in the kprobe attachment. So this issue would be solved by #3653 only.

[rafaeldtinoco]

Addressed by #3802