Skip to content

Commit

Permalink
Fix double-free in NewModuleFromBufferArgs
Browse files Browse the repository at this point in the history
In
0238ec3
the freeing of C strings was changed to use defers. This can cause a
double-free, which in the best case it will produce a crash. The reason
why this happens is that the memory address at `defer` time is captured
for later execution. If `KConfigFilePath` is less than 3, it was being
freed and set to NULL. Once the defer executes on function return, the
same address we already freed will be passed again.

We observed this while upgrading libbpfgo in Parca Agent
(parca-dev/parca-agent#1599).

Test Plan
=========

Verified it's a double free with ASAN

```
=================================================================
==171270==ERROR: AddressSanitizer: attempting double-free on 0x602000000010 in thread T14:
    #0 0x4d6e68 in __interceptor_free.part.0 asan_malloc_linux.cpp.o
    #1 0x3004be2 in _cgo_38fdf0a0bedf_Cfunc_free (/home/javierhonduco/code/parca-agent/dist/parca-agent+0x3004be2) (BuildId: aebc1e250e9da366a49de9206c528fb67b730e0b)
    #2 0x58bac3 in runtime.asmcgocall.abi0 runtime/asm_amd64.s:848

0x602000000010 is located 0 bytes inside of 1-byte region [0x602000000010,0x602000000011)
freed by thread T14 here:
    #0 0x4d6e68 in __interceptor_free.part.0 asan_malloc_linux.cpp.o
    #1 0x3004be2 in _cgo_38fdf0a0bedf_Cfunc_free (/home/javierhonduco/code/parca-agent/dist/parca-agent+0x3004be2) (BuildId: aebc1e250e9da366a49de9206c528fb67b730e0b)
    #2 0x58bac3 in runtime.asmcgocall.abi0 runtime/asm_amd64.s:848

previously allocated by thread T14 here:
    #0 0x4d7e37 in __interceptor_malloc (/home/javierhonduco/code/parca-agent/dist/parca-agent+0x4d7e37) (BuildId: aebc1e250e9da366a49de9206c528fb67b730e0b)
    #1 0x2ff3ff2 in _cgo_38fdf0a0bedf_Cfunc__Cmalloc (/home/javierhonduco/code/parca-agent/dist/parca-agent+0x2ff3ff2) (BuildId: aebc1e250e9da366a49de9206c528fb67b730e0b)
    #2 0x58bac3 in runtime.asmcgocall.abi0 runtime/asm_amd64.s:848
```

And that there are no issues with this patch applied, both while running
the Agent with and without ASAN as well as while running the cpu
profiling integration tests which exercise this code path.

Signed-off-by: Francisco Javier Honduvilla Coto <javierhonduco@gmail.com>
  • Loading branch information
javierhonduco committed May 9, 2023
1 parent 0120665 commit 6d77c03
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions libbpfgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ func NewModuleFromBufferArgs(args NewModuleArgs) (*Module, error) {
cBTFFilePath := C.CString(args.BTFObjPath)
defer C.free(unsafe.Pointer(cBTFFilePath))
cKconfigPath := C.CString(args.KConfigFilePath)
defer C.free(unsafe.Pointer(cKconfigPath))
cBPFObjName := C.CString(args.BPFObjName)
defer C.free(unsafe.Pointer(cBPFObjName))
cBPFBuff := unsafe.Pointer(C.CBytes(args.BPFObjBuff))
Expand All @@ -393,8 +394,6 @@ func NewModuleFromBufferArgs(args NewModuleArgs) (*Module, error) {
cKconfigPath = nil
}

defer C.free(unsafe.Pointer(cKconfigPath))

cOpts, errno := C.bpf_object_open_opts_new(cBTFFilePath, cKconfigPath, cBPFObjName)
if cOpts == nil {
return nil, fmt.Errorf("failed to create bpf_object_open_opts to %s: %w", args.BPFObjName, errno)
Expand Down

0 comments on commit 6d77c03

Please sign in to comment.