Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 27 additions & 23 deletions builtins/internal/winnet/winnet_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const (
// calling GetExtendedTcpTable / GetExtendedUdpTable. This cap is
// intentionally defined here (where the DLL calls live) so that the limit
// stays co-located with the code that enforces it.
MaxBufSize = 64 << 20 // 64 MiB
// 5 MiB is sufficient for any realistic socket table on Windows.
MaxBufSize = 5 << 20 // 5 MiB
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

P3 Badge Function comment above callExtendedTable references wrong constant name

Line 98 just below this hunk reads:

// grow-loop, capped at MaxWinBufSize.

MaxWinBufSize is the mirror constant defined in builtins/ss/ss.go. The constant actually enforced in this function is MaxBufSize (defined here in winnet_windows.go). The comment should read "capped at MaxBufSize".

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Increase Windows table cap to avoid silent socket loss

Reducing MaxBufSize to 5 MiB makes callExtendedTable fail with buffer size limit exceeded whenever GetExtendedTcpTable/GetExtendedUdpTable reports a required size above that value, which is valid API behavior (pdwSize is returned as the needed buffer size). This threshold is reachable on busy hosts (roughly >218k IPv4 TCP rows at 24 bytes each, or >93k IPv6 TCP rows at 56 bytes each). Since Collect() only returns an error if all sub-collections fail, a single oversized family can now be omitted from ss output without an error, leading to incomplete and misleading results.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i think this part is actually worth fixing:

Since Collect() only returns an error if all sub-collections fail, a single oversized family can now be omitted from ss output without an error, leading to incomplete and misleading results.

the numbers provided in the previous comment are theoretical only (imho).

cc @thieman

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will update it to fail entirely in that case, still seems unlikely we will go past the 5MB limit

)

var (
Expand All @@ -61,41 +62,44 @@ var tcpStateNames = map[uint32]string{

// Collect enumerates all TCP and UDP sockets on Windows via iphlpapi.dll.
// The narrow use of unsafe.Pointer is limited to two DLL call sites only.
// If every sub-collection fails, the first error is returned so callers can
// distinguish a real API failure from an empty socket table.
// If any sub-collection fails, an error is returned immediately so callers
// always receive complete data or a clear failure — never silent partial results.
func Collect() ([]SocketEntry, error) {
var out []SocketEntry
var firstErr error

collect := func(e []SocketEntry, err error) {
if err != nil {
if firstErr == nil {
firstErr = err
}
return
}
out = append(out, e...)
// TCP IPv4
e, err := collectTCP(afINET)
if err != nil {
return nil, fmt.Errorf("tcp4: %w", err)
}
out = append(out, e...)

// TCP IPv4
collect(collectTCP(afINET))
// TCP IPv6
collect(collectTCP(afINET6))
e, err = collectTCP(afINET6)
if err != nil {
return nil, fmt.Errorf("tcp6: %w", err)
}
out = append(out, e...)

// UDP IPv4
collect(collectUDP(afINET))
// UDP IPv6
collect(collectUDP(afINET6))
e, err = collectUDP(afINET)
if err != nil {
return nil, fmt.Errorf("udp4: %w", err)
}
out = append(out, e...)

// Return an error only when every collection failed and nothing was returned.
// Partial results (e.g. IPv6 unavailable) are still returned without error.
if len(out) == 0 && firstErr != nil {
return nil, firstErr
// UDP IPv6
e, err = collectUDP(afINET6)
if err != nil {
return nil, fmt.Errorf("udp6: %w", err)
}
out = append(out, e...)

return out, nil
}

// callExtendedTable calls GetExtendedTcpTable or GetExtendedUdpTable with a
// grow-loop, capped at MaxWinBufSize. Returns the raw buffer on success.
// grow-loop, capped at MaxBufSize. Returns the raw buffer on success.
func callExtendedTable(proc *syscall.Proc, af, tableClass uintptr) ([]byte, error) {
size := uint32(4096)
for {
Expand Down
7 changes: 1 addition & 6 deletions builtins/ss/ss.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
// macOS: sysctl returns a bounded []byte. Every offset dereference is
// bounds-checked against len(data) before reading.
//
// Windows: the DLL grow-loop is capped at MaxWinBufSize (64 MiB).
// Windows: the DLL grow-loop is capped at winnet.MaxBufSize (5 MiB).
// unsafe.Pointer is used only to pass &buf[0] to the DLL call; the
// returned data is parsed entirely with encoding/binary.LittleEndian.
package ss
Expand All @@ -115,11 +115,6 @@ var Cmd = builtins.Command{
// MaxLineBytes is the per-line buffer cap for the Linux /proc/net/ scanner.
const MaxLineBytes = 1 << 20 // 1 MiB

// MaxWinBufSize is the maximum buffer size used by the Windows grow-loop
// when calling GetExtendedTcpTable / GetExtendedUdpTable. This must match
// winnet.MaxBufSize; the winnet package owns the authoritative value.
const MaxWinBufSize = 64 << 20 // 64 MiB — keep in sync with winnet.MaxBufSize

// socketType identifies the protocol family of a socket entry.
type socketType int

Expand Down
Loading