google / google/gopacket

pcap: OpenOfflineFile appears unsafe on POSIX

Open
#719 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Go
Stars
6.8k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

I was looking at pcap for use in a project, and I think I've noticed two issues with pcap.OpenOfflineFile on POSIX:

1. After calling `file.Fd()` here:

https://github.com/google/gopacket/blob/0ad7f2610e344e58c1c95e2adda5c3258da8e97b/pcap/pcap_unix.go#L703

there are no more uses of `file`. So as far as the garbage collector is concerned, if it can't find a live reference to `file` anywhere else (e.g., a caller's stack frame), then `file` might be GC'd. This would then cause its finalizer to run, and close the underlying file descriptor.

In particular, if the program opens any other files in the mean time, the file descriptor might be reused and now referring to an entirely different file.

Go developers are required to use `runtime.KeepAlive(file)` to prevent GC from doing this. (E.g., see golang/go#34810.)

2. Calling `C.fdopen(fd)` passes ownership of `fd` to the `FILE*`. Once the `FILE*` is closed via `fclose` (which I'm assuming pcap handles calling eventually), the FD might be double closed again when the `os.File` is closed.

To prevent this, you probably want to use `C.dup` to duplicate a new file descriptor that can be handed off to `C.fdopen`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.