Skip to content

ipc, conn: resource-leak close file descriptors on error paths#147

Open
fereidani wants to merge 4 commits into
WireGuard:masterfrom
fereidani:ipc-conn-close-on-error-paths
Open

ipc, conn: resource-leak close file descriptors on error paths#147
fereidani wants to merge 4 commits into
WireGuard:masterfrom
fereidani:ipc-conn-close-on-error-paths

Conversation

@fereidani

Copy link
Copy Markdown

Hi,

I've built a static analyzer for Go called Ghoul, and I ran it against the wireguard-go codebase. It flagged a few error paths where a file descriptor is opened but never closed.

Ghoul reports:

ipc/uapi_linux.go:77:3: [resource-leak] resource from net.FileListener allocated at ipc/uapi_linux.go:56 never closed (confidence: 6.4/10, CWE-404)
ipc/uapi_linux.go:89:3: [resource-leak] resource from net.FileListener allocated at ipc/uapi_linux.go:56 never closed (confidence: 6.4/10, CWE-404)
ipc/uapi_linux.go:95:3: [resource-leak] resource from net.FileListener allocated at ipc/uapi_linux.go:56 never closed (confidence: 6.4/10, CWE-404)
ipc/uapi_unix.go:56:3: [resource-leak] resource from net.Dial allocated at ipc/uapi_unix.go:55 never closed (confidence: 6.4/10, CWE-404)

This series closes each leaked descriptor on its error path:

  • ipc/uapi_unix.go: close the unix connection opened for the socket liveness probe, and close the listener when duplicating its file descriptor fails.
  • ipc/uapi_linux.go: close the listener if inotify initialization, watch addition, or the rwcancel helper fails; close the inotify file descriptor as well when watch addition fails.
  • conn/bind_std.go: close the listening socket in listenNet when resolving its local address fails.

Each fix is focused on a single path and lives in its own commit.

UAPIOpen dials the socket only to check whether it is in use, but it
left the established connection open, leaking a file descriptor on every
attempt that found the socket busy.

Signed-off-by: Khashayar Fereidani <info@fereidani.com>
If listener.File() returns an error, the underlying unix listener is
left open, leaking its file descriptor. Close it on the failure path.

Signed-off-by: Khashayar Fereidani <info@fereidani.com>
UAPIListen creates a listener from the file descriptor before setting
up inotify. If inotify initialization, watch addition, or the rwcancel
helper fails, the listener is returned to the caller neither closed nor
handed off, leaking its file descriptor. The inotify file descriptor is
likewise leaked when watch addition fails. Close them on each path.

Signed-off-by: Khashayar Fereidani <info@fereidani.com>
listenNet resolves the local address after opening the socket. If
resolution fails, the socket is returned neither closed nor handed off,
leaking its file descriptor.

Signed-off-by: Khashayar Fereidani <info@fereidani.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants