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

Fix undefined behaviour in variadic argument handling #248

Merged
merged 2 commits into from
Sep 21, 2022

Conversation

javierhonduco
Copy link
Contributor

We were seeing some crashes in Parca Agent (parca-dev/parca-agent#817) when running on libbpfgo v0.4.0-libbpf-1.0.0.

fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0x3c pc=0x18724f1]

runtime stack:
runtime.throw({0x1da3dc1?, 0x0?})
	runtime/panic.go:992 +0x71
runtime.sigpanic()
	runtime/signal_unix.go:802 +0x3a9
[...]

Under gdb, the top of the stack seemed to be around the libbpf printing facilities:

(gdb) bt
#0  0x00000000018724f1 in __strstr_sse2_unaligned ()
#1  0x00000000017c4156 in libbpf_print_fn ()
#2  0x00000000017cb31c in libbpf_print (level=(unknown: 0x3c), level@entry=LIBBPF_WARN, format=0x205ad64 "cgroup") at libbpf.c:103
#3  0x00000000017debc7 in bpf_object__init_user_maps (obj=0x7fff9c0012a0, strict=<optimized out>) at libbpf.c:2004
#4  bpf_object__init_maps (obj=obj@entry=0x7fff9c0012a0, opts=opts@entry=0xc0000fe280) at libbpf.c:2617
#5  0x00000000017cdfe7 in bpf_object_open (path=<optimized out>, path@entry=0x0, obj_buf=0x7fff9c000b90, obj_buf_sz=1792, opts=0xc0000fe280) at libbpf.c:7410
#6  0x00000000017ce1ad in bpf_object__open_mem (obj_buf=0x205ad64, obj_buf_sz=103, opts=0x7fffa6ffbd50) at libbpf.c:7472
#7  0x00000000017c49f4 in _cgo_b2805b411fbe_C2func_bpf_object__open_mem ()
#8  0x000000000046c924 in runtime.asmcgocall () at runtime/asm_amd64.s:821
#9  0x0000000000000004 in ?? ()
#10 0x000000c00063a7d0 in ?? ()
#11 0x0000000000000001 in ?? ()
#12 0x0000000000000000 in ?? ()

Which led me to the variadic arguments being reused after iterating over them. Also added the missing va_ends before returning, as per the UNIX spec

Corresponding va_start() and va_end() macro calls must be in the same function.

https://www.ibm.com/docs/en/zos/2.3.0?topic=lf-va-arg-va-copy-va-end-va-start-access-function-arguments

Signed-off-by: Francisco Javier Honduvilla Coto javierhonduco@gmail.com

@javierhonduco javierhonduco force-pushed the variadic-args-fix branch 5 times, most recently from 0bba9e4 to 5302dc4 Compare September 21, 2022 12:33
Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>
Copy link
Member

@geyslan geyslan left a comment

Choose a reason for hiding this comment

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

As tests passed and based on the stdarg man, this LGTM. Thanks @javierhonduco

va_end()
       Each invocation of va_start() must be matched by a corresponding
       invocation of va_end() in the same function.  After the call
       va_end(ap) the variable ap is undefined.  Multiple traversals of
       the list, each bracketed by va_start() and va_end() are possible.
       va_end() may be a macro or a function.

I would only add the above reference.

--- EDIT:

Done.

libbpfgo.go Outdated Show resolved Hide resolved
Add stdarg reference in comments. If not needed in the future, these comments can be removed.
@grantseltzer grantseltzer merged commit 245a1a1 into aquasecurity:main Sep 21, 2022
@javierhonduco javierhonduco deleted the variadic-args-fix branch September 22, 2022 09:05
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.

3 participants