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

sys: avoid zero fd #454

Merged
merged 2 commits into from
Oct 25, 2021
Merged

sys: avoid zero fd #454

merged 2 commits into from
Oct 25, 2021

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Oct 21, 2021

Upstream libbpf recently added code to avoid fds below three 1. The BPF
UAPI assumes in many places that a 0 value for fd means no argument was
provided. If stdin is closed by the program we can get a situation where
a BPF syscall returns a zero fd for a map, program or similar.

Add a similar mitigation, but restrict it to the zero value. This is because
the Go runtime already assumes that stderr is at the canonical fd 2:

Note that the Go runtime writes to standard error for panics and crashes;
closing Stderr may cause those messages to go elsewhere, perhaps to a
file opened later.

This means that a Go program fiddling with stdin, stdout, stderr already has
to at least account for this.

Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Nice! Why don't we return *sys.FD everywhere an fd int or uintptr is returned instead? I mean primarily sys.BPF, but the perf epoll stuff too, although those fds never go back into bpf(). Feels like we might want to avoid passing around bare fds without finalizers, they're pretty easy to leak.

internal/sys/fd.go Show resolved Hide resolved
link/kprobe.go Show resolved Hide resolved
New versions of the library have qt.IsError.
Upstream libbpf recently added code to avoid fds below three [1]. The BPF
UAPI assumes in many places that a 0 value for fd means no argument was
provided. If stdin is closed by the program we can get a situation where
a BPF syscall returns a zero fd for a map, program or similar.

Add a similar mitigation, but restrict it to the zero value. This is because
the Go runtime already assumes that stderr is at the canonical fd [2]:

    Note that the Go runtime writes to standard error for panics and crashes;
    closing Stderr may cause those messages to go elsewhere, perhaps to a
    file opened later.

This means that a Go program fiddling with stdin, stdout, stderr already has
to at least account for this.

[1]: https://lore.kernel.org/bpf/20211020191526.2306852-5-memxor@gmail.com/
[2]: https://pkg.go.dev/os#pkg-variables
@ti-mo ti-mo merged commit d4a68b0 into master Oct 25, 2021
@ti-mo ti-mo deleted the lmb/fd-non-zero branch October 25, 2021 18:22
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.

None yet

2 participants