Panic on access to MIB_TCPTABLE2 access when built against nightly (only)

[zeenix]

Summary

I'm seeing a strange issue where the code that's been working for a while, started to panic but only if built with nightly and not with stable Rust.

Crate manifest

[package]
name = "zbus"
version = "4.0.0"
authors = ["Zeeshan Ali Khan <zeeshanak@gnome.org>"]
edition = "2021"
rust-version = "1.75"

description = "API for D-Bus communication"
repository = "https://github.com/dbus2/zbus/"
keywords = ["D-Bus", "DBus", "IPC"]
license = "MIT"
categories = ["os::unix-apis"]
readme = "README.md"

[features]
default = ["async-io"]
uuid = ["zvariant/uuid"]
url = ["zvariant/url"]
time = ["zvariant/time"]
chrono = ["zvariant/chrono"]
# Enables ser/de of `Option<T>` as an array of 0 or 1 elements.
option-as-array = ["zvariant/option-as-array"]
# Enables API that is only needed for bus implementations (enables `p2p`).
bus-impl = ["p2p"]
# Enables API that is only needed for peer-to-peer (p2p) connections.
p2p = []
windows-gdbus = []
async-io = [
  "dep:async-io",
  "async-executor",
  "async-task",
  "async-lock",
  "async-fs",
  "blocking",
  "futures-util/io",
]
tokio = ["dep:tokio"]
vsock = ["dep:vsock", "dep:async-io"]
tokio-vsock = ["dep:tokio-vsock", "tokio"]

[dependencies]
serde = { version = "1.0", features = ["derive"] }
serde_repr = "0.1.9"
zvariant = { path = "../zvariant", version = "4.0.0", default-features = false, features = [
  "enumflags2",
] }
zbus_names = { path = "../zbus_names", version = "3.0" }
zbus_macros = { path = "../zbus_macros", version = "=4.0.0" }
enumflags2 = { version = "0.7.7", features = ["serde"] }
derivative = "2.2"
async-io = { version = "2.2.2", optional = true }
futures-core = "0.3.25"
futures-sink = "0.3.25"
futures-util = { version = "0.3.25", default-features = false, features = [
  "sink",
  "std",
] }
async-lock = { version = "3.0.0", optional = true }
async-broadcast = "0.7.0"
async-executor = { version = "1.5.0", optional = true }
blocking = { version = "1.0.2", optional = true }
async-task = { version = "4.3.0", optional = true }
hex = "0.4.3"
ordered-stream = "0.2"
rand = "0.8.5"
sha1 = { version = "0.10.5", features = ["std"] }
event-listener = "5.0.0"
static_assertions = "1.1.0"
async-trait = "0.1.58"
async-fs = { version = "2.0.0", optional = true }
# FIXME: We should only enable process feature for Mac OS. See comment on async-process below for why we can't.
tokio = { version = "1", optional = true, features = [
  "rt",
  "net",
  "time",
  "fs",
  "io-util",
  "process",
  "sync",
  "tracing",
] }
tracing = "0.1.37"
vsock = { version = "0.4.0", optional = true }
tokio-vsock = { version = "0.4", optional = true }
xdg-home = "1.0.0"

[target.'cfg(windows)'.dependencies]
windows-sys = { version = "0.52", features = [
  "Win32_Foundation",
  "Win32_Security_Authorization",
  "Win32_System_Memory",
  "Win32_Networking",
  "Win32_Networking_WinSock",
  "Win32_NetworkManagement",
  "Win32_NetworkManagement_IpHelper",
  "Win32_System_Threading",
] }
uds_windows = "1.1.0"

[target.'cfg(unix)'.dependencies]
nix = { version = "0.27", default-features = false, features = [
  "socket",
  "uio",
  "user",
] }

[target.'cfg(target_os = "macos")'.dependencies]
# FIXME: This should only be enabled if async-io feature is enabled but currently
# Cargo doesn't provide a way to do that for only specific target OS: https://github.com/rust-lang/cargo/issues/1197.
async-process = "2.0.0"

[target.'cfg(any(target_os = "macos", windows))'.dependencies]
async-recursion = "1.0.0"

[dev-dependencies]
zbus_xml = { path = "../zbus_xml", version = "4.0.0" }
doc-comment = "0.3.3"
futures-util = "0.3.25" # activate default features
ntest = "0.9.0"
test-log = { version = "0.2.11", features = [
  "trace",
], default-features = false }
tokio = { version = "1", features = [
  "macros",
  "rt-multi-thread",
  "fs",
  "io-util",
  "net",
  "sync",
] }
async-std = { version = "1.12.0", features = ["attributes"] }
tracing-subscriber = { version = "0.3.16", features = [
  "env-filter",
  "fmt",
  "ansi",
], default-features = false }
tempfile = "3.3.0"

[package.metadata.docs.rs]
all-features = true
targets = ["x86_64-unknown-linux-gnu"]

Crate code

// A bit pressed for time at the moment so I can't create a self-contained reproducer at
// the moment. I will do so when I can but for now, I can copy&paste the relevant zbus code here:

// Get the process ID of the local socket address
// TODO: add ipv6 support
pub fn socket_addr_get_pid(addr: &SocketAddr) -> Result<u32, Error> {
    let mut len = 4096;
    let mut tcp_table = vec![];
    let res = loop {
        tcp_table.resize(len as usize, 0);
        let res = unsafe {
            GetTcpTable2(
                tcp_table.as_mut_ptr().cast::<MIB_TCPTABLE2>(),
                ptr::addr_of_mut!(len),
                0,
            )
        };
        if res != ERROR_INSUFFICIENT_BUFFER {
            break res;
        }
    };
    if res != NO_ERROR {
        return Err(Error::last_os_error());
    }

    let tcp_table = tcp_table.as_mut_ptr().cast::<MIB_TCPTABLE2>();
    let num_entries = unsafe { (*tcp_table).dwNumEntries };
    for i in 0..num_entries {
        //
        //
        // This is the line that panics:
        //
        //
        let entry = unsafe { (*tcp_table).table.get_unchecked(i as usize) };

        let port = (entry.dwLocalPort & 0xFFFF) as u16;
        let port = u16::from_be(port);

        if entry.dwState == MIB_TCP_STATE_ESTAB as u32
            && u32::from_be(entry.dwLocalAddr) == INADDR_LOOPBACK
            && u32::from_be(entry.dwRemoteAddr) == INADDR_LOOPBACK
            && port == addr.port()
        {
            return Ok(entry.dwOwningPid);
        }
    }

    Err(Error::new(ErrorKind::Other, "PID of TCP address not found"))
}

// Full code here: https://github.com/dbus2/zbus/blob/af493c4a99310248e115dd3ccbd06a503444eafb/zbus/src/win32.rs#L194

[Nerixyz]

The slice of MIB_TCPTABLE2 is declared with length 1, and the length is checked here (added about a month ago). You'd probably need to work with tcp_table->table as a pointer.

[zeenix]

@Nerixyz Firstly, thanks for the very quick response and explanation.

The slice of MIB_TCPTABLE2 is declared with length 1, and the length is checked here (added about a month ago).

Interesting. While I didn't write the relevant code in zbus, I did notice that it's based on example C code from the official win32 docs.

You'd probably need to work with tcp_table->table as a pointer.

Not sure I understand. 🤔 The tcp_table->table field is array with entries declared to be MIB_TCPROW2 structs (and not anything that could be a pointer). If I understand correctly, the number of entries in the slice are supposed to match tcp_table->dwNumEntries.

[Nerixyz]

Not sure I understand. 🤔 The tcp_table->table field is array with entries declared to be MIB_TCPROW2 structs (and not anything that could be a pointer). If I understand correctly, the number of entries in the slice are supposed to match tcp_table->dwNumEntries.

Yea, in C, table is a flexible array member. These don't really map to anything in Rust, so they're generated as [T; 1]. So the length and data are basically stored in-place.

Note that dwNumEntries number of entries, while SizePointer (second argument to GetTcpTable2) is the size of the buffer, of the tcp_table, in bytes.

[zeenix]

Not sure I understand. 🤔 The tcp_table->table field is array with entries declared to be MIB_TCPROW2 structs (and not anything that could be a pointer). If I understand correctly, the number of entries in the slice are supposed to match tcp_table->dwNumEntries.

Yea, in C, table is a flexible array member. These don't really map to anything in Rust, so they're generated as [T; 1]. So the length and data are basically stored in-place.

Hmm.. are you saying that while the struct doesn't represent this in any way, there are actually multiple entries there, so I can just ptr.add(i) to iterate over the entries? 😮

[zeenix]

Hmm.. are you saying that while the struct doesn't represent this in any way, there are actually multiple entries there, so I can just ptr.add(i) to iterate over the entries? 😮

I guess so, cause this seems to help:

diff --git a/zbus/src/win32.rs b/zbus/src/win32.rs
index 24e1236d..f75e31e1 100644
--- a/zbus/src/win32.rs
+++ b/zbus/src/win32.rs
@@ -214,7 +214,7 @@ pub fn socket_addr_get_pid(addr: &SocketAddr) -> Result<u32, Error> {
     let tcp_table = tcp_table.as_mut_ptr().cast::<MIB_TCPTABLE2>();
     let num_entries = unsafe { (*tcp_table).dwNumEntries };
     for i in 0..num_entries {
-        let entry = unsafe { (*tcp_table).table.get_unchecked(i as usize) };
+        let entry = unsafe { *(*tcp_table).table.as_ptr().add(i as usize) };
         let port = (entry.dwLocalPort & 0xFFFF) as u16;
         let port = u16::from_be(port);

If I'm right, please feel free to close this issue. Thanks for your help in either case.

[zeenix]

@Nerixyz Thanks again. I'm close this now. It would be great if the documentation could be helpful here but I'm guessing that's difficult with generated docs and a huge API surface.