Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libbpfgo: make AttachTracepoint() signature 1:1 with libbpf #77

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

geyslan
Copy link
Member

@geyslan geyslan commented Sep 12, 2021

These changes, along with some refactoring, also update the libbpfgo_test.go.

Fixes: #75

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Sep 13, 2021

I have tested this with:

        SEC("tp/syscalls/sys_enter_sync")
        int tracepoint__sys_enter_sync(struct trace_event_raw_sys_enter *args)
        {
            return dealwithit(args);
        }
	// attach to BPF program to kprobe
	//_, err = bpfProgKsysSync.AttachTracepoint("syscalls:sys_enter_sync")
	_, err = bpfProgKsysSync.AttachTracepoint("syscalls", "sys_enter_sync")
	if err != nil {
		errExit(err)
	}

it works the same way (simplifying the split logic done within AttachTracepoint.

I think its okay to accept this (not that is changing anything meaningful but... it removes any doubts about category versus name.

@rafaeldtinoco
Copy link
Contributor

ORTHOGONAL to this change: tracepoint attachment does not seem to be working in older kernels (not sure if because of what I predicted: tracepoint and perf events attachment share same baseline functions for attachment).

Not sure if super important also as we will likely drop support for those older kernels soon.

GO code:

$ sudo ./mine-static
libbpf: failed to parse target BTF: -2
libbpf: failed to perform CO-RE relocations: -2
libbpf: failed to load object 'mine.bpf.o'
2021/09/13 10:59:30 error: /home/rafaeldtinoco/work/sources/ebpf/mine-portablebpf/mine.go:79 failed to load BPF object

Equivalent C code:

$ sudo ./mine-c-static
Foreground mode...<Ctrl-C> or or SIG_TERM to end it.
libbpf: loading object 'mine_bpf' from buffer
libbpf: elf: section(3) tracepoint/syscalls/sys_enter_sync, size 320, link 0, flags 6, type=1
libbpf: sec 'tracepoint/syscalls/sys_enter_sync': found program 'tracepoint__sys_enter_sync' at insn offset 0 (0 bytes), code size 40 insns (320 bytes)
libbpf: elf: section(4) .reltracepoint/syscalls/sys_enter_sync, size 16, link 22, flags 0, type=9
libbpf: elf: section(5) license, size 4, link 0, flags 3, type=1
libbpf: license of mine_bpf is GPL
libbpf: elf: section(6) .maps, size 24, link 0, flags 3, type=1
libbpf: elf: section(13) .BTF, size 26786, link 0, flags 0, type=1
libbpf: elf: section(15) .BTF.ext, size 348, link 0, flags 0, type=1
libbpf: elf: section(22) .symtab, size 288, link 1, flags 0, type=2
libbpf: looking for externs among 12 symbols...
libbpf: collected 0 externs total
libbpf: map 'events': at sec_idx 6, offset 0.
libbpf: map 'events': found type = 4.
libbpf: map 'events': found key_size = 4.
libbpf: map 'events': found value_size = 4.
libbpf: sec '.reltracepoint/syscalls/sys_enter_sync': collecting relocation for section(3) 'tracepoint/syscalls/sys_enter_sync'
libbpf: sec '.reltracepoint/syscalls/sys_enter_sync': relo #0: insn #31 against 'events'
libbpf: prog 'tracepoint__sys_enter_sync': found map 0 (events, sec 6, off 0) for insn #31
libbpf: loading kernel BTF '/usr/lib/debug/boot/vmlinux-4.15.0-156-generic': 0
libbpf: Kernel doesn't support BTF, skipping uploading it.
libbpf: map 'events': setting size to 4
libbpf: map 'events': created successfully, fd=3
libbpf: sec 'tracepoint/syscalls/sys_enter_sync': found 2 CO-RE relocations
libbpf: CO-RE relocating [0] struct task_struct: found target candidate [142] struct task_struct in [vmlinux]
libbpf: CO-RE relocating [0] struct task_struct: found target candidate [25201] struct task_struct in [vmlinux]
libbpf: prog 'tracepoint__sys_enter_sync': relo #0: kind <byte_off> (0), spec is [19] struct task_struct.loginuid.val (0:123:0 @ offset 2992)
libbpf: prog 'tracepoint__sys_enter_sync': relo #0: matching candidate #0 [142] struct task_struct.loginuid.val (0:109:0 @ offset 2824)
libbpf: prog 'tracepoint__sys_enter_sync': relo #0: matching candidate #1 [25201] struct task_struct.loginuid.val (0:109:0 @ offset 2824)
libbpf: prog 'tracepoint__sys_enter_sync': relo #0: patched insn #15 (ALU/ALU64) imm 2992 -> 2824
libbpf: prog 'tracepoint__sys_enter_sync': relo #1: kind <byte_off> (0), spec is [19] struct task_struct.comm (0:102 @ offset 2792)
libbpf: prog 'tracepoint__sys_enter_sync': relo #1: matching candidate #0 [142] struct task_struct.comm (0:90 @ offset 2640)
libbpf: prog 'tracepoint__sys_enter_sync': relo #1: matching candidate #1 [25201] struct task_struct.comm (0:90 @ offset 2640)
libbpf: prog 'tracepoint__sys_enter_sync': relo #1: patched insn #22 (ALU/ALU64) imm 2792 -> 2640
Tracing... Hit Ctrl-C to end.

GO code works in newer kernels:

$ sudo ./mine-static
Listening for events, <Ctrl-C> or or SIG_TERM to end it.
sync (pid: 3932861) (loginuid: 1000)

@CLAassistant
Copy link

CLAassistant commented Sep 14, 2021

CLA assistant check
All committers have signed the CLA.

libbpfgo.go Outdated
Comment on lines 763 to 764
C.free(unsafe.Pointer(tpCategory))
C.free(unsafe.Pointer(tpName))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: what was wrong with freeing the C strings here that we need to defer it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a nitpicking, perhaps trying to use the Golang idioms. But this change is not mandatory. If rejected I can get rid of it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well in this case, we can just free the allocated objects right after we use them, and it seems to be clearer (at least for me).
I would use defer when there is an operation (like closing a file) that can't happen right away, and I want to ensure that will happen in any return path of the function.

libbpfgo.go Outdated
}

bpfLink := &BPFLink{
link: link,
prog: p,
linkType: Tracepoint,
eventName: tp,
eventName: fmt.Sprintf("%s:%s", category, name),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it is ok if we just use name here so it will match the above error message where we also return name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Consider it done.

@yanivagman
Copy link
Collaborator

yanivagman commented Sep 26, 2021

ORTHOGONAL to this change: tracepoint attachment does not seem to be working in older kernels (not sure if because of what I predicted: tracepoint and perf events attachment share same baseline functions for attachment).

Not sure if super important also as we will likely drop support for those older kernels soon.

GO code:

$ sudo ./mine-static
libbpf: failed to parse target BTF: -2
libbpf: failed to perform CO-RE relocations: -2
libbpf: failed to load object 'mine.bpf.o'
2021/09/13 10:59:30 error: /home/rafaeldtinoco/work/sources/ebpf/mine-portablebpf/mine.go:79 failed to load BPF object

@rafaeldtinoco I'm not sure that this problem has anything to do with tracepoints. It seems to me more of a CO-RE problem (libbpf: failed to parse target BTF: -2). I believe that your mine-static program is CO-RE, while you are running on a kernel which doesn't support BTF (as can be seen in the C equivalent code: libbpf: Kernel doesn't support BTF, skipping uploading it.). Can you please try to compile it "non-CO-RE" and try again?

@rafaeldtinoco
Copy link
Contributor

rafaeldtinoco commented Sep 27, 2021

@rafaeldtinoco I'm not sure that this problem has anything to do with tracepoints. It seems to me more of a CO-RE problem (libbpf: failed to parse target BTF: -2). I believe that your mine-static program is CO-RE, while you are running on a kernel which doesn't support BTF (as can be seen in the C equivalent code: libbpf: Kernel doesn't support BTF, skipping uploading it.). Can you please try to compile it "non-CO-RE" and try again?

No, actually there is a bug in a logic I added to NewModuleFromFileArgs(). If there are no BTF files specified, I specify "/sys/kernel/btf/vmlinux". In this particular environment my BTF is coming from the debug package (which is hardcoded in libbpf and should work). With this change:

diff --git a/libbpfgo.go b/libbpfgo.go
index 5bc8a80..02093a1 100644
--- a/libbpfgo.go
+++ b/libbpfgo.go
@@ -348,15 +348,15 @@ func NewModuleFromFileArgs(args NewModuleArgs) (*Module, error) {
        if err := bumpMemlockRlimit(); err != nil {
                return nil, err
        }
-       if args.BTFObjPath == "" {
-               args.BTFObjPath = "/sys/kernel/btf/vmlinux"
-       }
-       btfFile := C.CString(args.BTFObjPath)
+       //if args.BTFObjPath == "" {
+       //args.BTFObjPath = "/sys/kernel/btf/vmlinux"
+       //}
+       //btfFile := C.CString(args.BTFObjPath)
        bpfFile := C.CString(args.BPFObjPath)

        opts := C.struct_bpf_object_open_opts{}
        opts.sz = C.sizeof_struct_bpf_object_open_opts
-       opts.btf_custom_path = btfFile // instruct libbpf to use user provided kernel BTF file
+       //opts.btf_custom_path = btfFile // instruct libbpf to use user provided kernel BTF file

        if strings.Compare(args.KConfigFilePath, "") != 0 {
                kConfigFile := C.CString(args.KConfigFilePath)
@@ -370,7 +370,7 @@ func NewModuleFromFileArgs(args NewModuleArgs) (*Module, error) {
        }

        C.free(unsafe.Pointer(bpfFile))
-       C.free(unsafe.Pointer(btfFile))
+       //C.free(unsafe.Pointer(btfFile))

        return &Module{
                obj: obj,

It works again. I have opened #81 to fix this as we shouldn't specify a custom BTF file if there isn't one.

@geyslan geyslan force-pushed the 75-attach-tp-signature branch from 4ac9860 to 04fa5a7 Compare September 27, 2021 19:04
@geyslan geyslan requested a review from yanivagman September 27, 2021 19:05
@yanivagman yanivagman merged commit d60c3bb into aquasecurity:main Sep 27, 2021
geyslan added a commit to geyslan/kubearmor-libbpf that referenced this pull request Sep 28, 2021
@geyslan geyslan deleted the 75-attach-tp-signature branch March 31, 2023 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracepoint naming isn't libbpf 1:1
4 participants