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

feat(lib): use memfd on linux instead of dumping libddwaf.so in /tmp #106

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

eliottness
Copy link
Contributor

What

This PR refactors the code in internal/lib to accommodate two implementations of the DumpEmbeddedWAF() function. One for linux using memfd, ultimately calling dlopen("/proc/self/fd/{fd}"), and one for darwin still creating a temporary file and calling dlopen("/tmp/libddwaf-*.so").

Side quest: some function comments were not starting by the name of the function, fixed it.

Motivation

Being able to use dd-trace-go on full read-only filesystems. On linux only for now but still

@eliottness eliottness requested a review from a team as a code owner September 2, 2024 15:57
Copy link
Collaborator

@Julio-Guerra Julio-Guerra left a comment

Choose a reason for hiding this comment

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

Great idea 🎉

internal/lib/dump_waf_linux.go Outdated Show resolved Hide resolved
internal/lib/dump_waf_linux.go Outdated Show resolved Hide resolved
Copy link
Contributor

@keisku keisku left a comment

Choose a reason for hiding this comment

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

Can you update this documentation as well? Thank you!

go-libddwaf/README.md

Lines 128 to 137 in 48f7206

### CGO-less C Bindings
This library uses [purego](https://github.com/ebitengine/purego) to implement C bindings without requiring use of CGO at compilation time. The high-level workflow
is to embed the C shared library using `go:embed`, dump it into a file, open the library using `dlopen`, load the
symbols using `dlsym`, and finally call them.
> :warning: Keep in mind that **purego only works on linux/darwin for amd64/arm64 and so does go-libddwaf.**
Another requirement of `libddwaf` is to have a FHS filesystem on your machine and, for linux, to provide `libc.so.6`,
`libpthread.so.0`, and `libdl.so.2` as dynamic libraries.

@eliottness eliottness enabled auto-merge (squash) September 3, 2024 09:03
Copy link
Contributor

@keisku keisku left a comment

Choose a reason for hiding this comment

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

I don't have background context of errors that have been ignored.
Logging or returning an error should help support team in the future!

// The caller is responsible for calling wafDl.Close on the returned object once they
// are done with it so that associated resources can be released.
func NewWafDl() (dl *WafDl, err error) {
file, err := lib.DumpEmbeddedWAF()
path, closer, err := lib.DumpEmbeddedWAF()
if err != nil {
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not ignore this err?

Suggested change
return
return nil, fmt.Errorf("write an embedded WAF library: %w", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sh*t. Should have disabled auto-merge when I saw your comments. Don't worry I will do it in another PR

}
}()

var handle uintptr
if handle, err = purego.Dlopen(file, purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil {
if handle, err = purego.Dlopen(path, purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil {
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return
return nil, fmt.Errorf("dlopen: %w", err)

@eliottness eliottness merged commit 5eb76be into main Sep 4, 2024
130 checks passed
@eliottness eliottness deleted the eliott.bouhana/memfd branch September 4, 2024 07:49
eliottness pushed a commit that referenced this pull request Sep 4, 2024
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