usbip: fix correctness findings from protocol audit

Windows export now reports the real USB link speed, probed from the
parent hub (IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX and _V2 for
SuperSpeedPlus), so SuperSpeed devices route to the correct root-hub
speed domain instead of advertising speed=0.

- protocol: pin DeviceInfoTruncated/DeviceInterface wire sizes with
  two-sided compile-time assertions so a struct change fails the build
  instead of silently mis-bounding the reader
- server: bound inbound connections with a handshake read deadline and a
  per-iteration idle deadline on the control loop, plus write deadlines
  on control writes; clear the deadline before the conn becomes a data
  session
- server: serialize import reservation under reconcileAccess so a reserve
  cannot interleave a reconcile pass that would release a busy device
- data: validate CMD_SUBMIT iso descriptor offset/length against the
  transfer buffer before forwarding to a platform engine
- darwin: make darwinUSBHostDevice.Close idempotent via sync.Once to
  avoid a double close/free under concurrent shutdown
- windows: guard windowsExport.device with a mutex and hand the claimed
  handle to a single closer

common/vboxusb/pnp_windows.go

@@ -41,6 +41,7 @@ type USBDeviceInfo struct {
 	Address     uint32 // device address on the bus (port path leaf)
 	BusID       string // "<bus>-<address>"
 	DeviceClass uint8
+	Speed       DeviceSpeed
 }
 
 // EnumerateUSBDevices walks GUID_DEVINTERFACE_USB_DEVICE and returns
@@ -61,6 +62,9 @@ func EnumerateUSBDevices() ([]USBDeviceInfo, error) {
 	}
 	defer devInfo.Close()
 
+	probe := newHubSpeedProbe()
+	defer probe.close()
+
 	var out []USBDeviceInfo
 	for i := 0; ; i++ {
 		data, err := windows.SetupDiEnumDeviceInfo(devInfo, i)
@@ -89,6 +93,7 @@ func EnumerateUSBDevices() ([]USBDeviceInfo, error) {
 			info.Address = toUint32(addressValue)
 		}
 		info.BusID = strconv.FormatUint(uint64(info.BusNumber), 10) + "-" + strconv.FormatUint(uint64(info.Address), 10)
+		info.Speed = probe.speedOf(devInfo, data, info.Address)
 		out = append(out, info)
 	}
 	return out, nil

common/vboxusb/speed_windows.go

@@ -0,0 +1,195 @@
+//go:build windows
+
+package vboxusb
+
+import (
+	"encoding/binary"
+
+	"golang.org/x/sys/windows"
+)
+
+// DeviceSpeed is the negotiated USB link speed of a device, independent of the
+// USB/IP wire encoding. The export host maps it to the protocol speed code.
+type DeviceSpeed uint8
+
+const (
+	SpeedUnknown DeviceSpeed = iota
+	SpeedLow
+	SpeedFull
+	SpeedHigh
+	SpeedSuper
+	SpeedSuperPlus
+)
+
+// GUID_DEVINTERFACE_USB_HUB.
+var usbHubInterfaceGUID = windows.GUID{
+	Data1: 0xf18a0e88,
+	Data2: 0xc30c,
+	Data3: 0x11d0,
+	Data4: [8]byte{0x88, 0x15, 0x00, 0xa0, 0xc9, 0x06, 0xbe, 0xd8},
+}
+
+// DEVPKEY_Device_Parent: the parent device instance id (the hub a device
+// hangs off). Reported as a DEVPROP_TYPE_STRING.
+var devpkeyDeviceParent = windows.DEVPROPKEY{
+	FmtID: windows.DEVPROPGUID{
+		Data1: 0x4340a6c5,
+		Data2: 0x93fa,
+		Data3: 0x4706,
+		Data4: [8]byte{0x97, 0x2c, 0x7b, 0x64, 0x80, 0x08, 0xa5, 0xa7},
+	},
+	PID: 8,
+}
+
+const (
+	ioctlUSBGetNodeConnectionInformationEx   uint32 = 0x0022_0448
+	ioctlUSBGetNodeConnectionInformationExV2 uint32 = 0x0022_048C
+
+	// Offset of the Speed byte inside USB_NODE_CONNECTION_INFORMATION_EX:
+	// ConnectionIndex(4) + USB_DEVICE_DESCRIPTOR(18) + CurrentConfigurationValue(1).
+	nodeConnInfoExSpeedOffset = 23
+	// Fixed part is 36 bytes; the IOCTL also appends one USB_PIPE_INFO per open
+	// pipe and fails if the buffer is too small, so allow for a fully configured
+	// device's pipe list.
+	nodeConnInfoExBufferSize = 2048
+
+	// USB_DEVICE_SPEED values reported by the EX IOCTL.
+	usbDeviceSpeedLow   = 0
+	usbDeviceSpeedFull  = 1
+	usbDeviceSpeedHigh  = 2
+	usbDeviceSpeedSuper = 3
+
+	// USB_NODE_CONNECTION_INFORMATION_EX_V2: ConnectionIndex(4) + Length(4) +
+	// SupportedUsbProtocols(4) + Flags(4).
+	nodeConnInfoExV2Size      = 16
+	nodeConnInfoExV2FlagsOff  = 12
+	nodeConnInfoExV2LengthOff = 4
+	// USB_NODE_CONNECTION_INFORMATION_EX_V2_FLAGS bit
+	// DeviceIsOperatingAtSuperSpeedPlusOrHigher.
+	nodeConnInfoExV2FlagSuperSpeedPlus = 0x4
+)
+
+// hubSpeedProbe resolves the negotiated link speed of devices by querying their
+// parent hub. Open hub handles are cached for the lifetime of one enumeration;
+// a nil/InvalidHandle entry caches a failure so it is not retried per device.
+type hubSpeedProbe struct {
+	hubs map[string]windows.Handle
+}
+
+func newHubSpeedProbe() *hubSpeedProbe {
+	return &hubSpeedProbe{hubs: make(map[string]windows.Handle)}
+}
+
+func (p *hubSpeedProbe) close() {
+	for _, handle := range p.hubs {
+		if handle != windows.InvalidHandle {
+			_ = windows.CloseHandle(handle)
+		}
+	}
+	p.hubs = nil
+}
+
+// speedOf returns the device's link speed, or SpeedUnknown if the parent hub
+// could not be opened or did not answer. port is the hub port index
+// (SPDRP_ADDRESS) the device is attached to.
+func (p *hubSpeedProbe) speedOf(devInfo windows.DevInfo, data *windows.DevInfoData, port uint32) DeviceSpeed {
+	hub := p.parentHub(devInfo, data)
+	if hub == windows.InvalidHandle {
+		return SpeedUnknown
+	}
+	return querySpeed(hub, port)
+}
+
+func (p *hubSpeedProbe) parentHub(devInfo windows.DevInfo, data *windows.DevInfoData) windows.Handle {
+	parentValue, err := windows.SetupDiGetDeviceProperty(devInfo, data, &devpkeyDeviceParent)
+	if err != nil {
+		return windows.InvalidHandle
+	}
+	parentID, isString := parentValue.(string)
+	if !isString || parentID == "" {
+		return windows.InvalidHandle
+	}
+	paths, err := windows.CM_Get_Device_Interface_List(parentID, &usbHubInterfaceGUID, windows.CM_GET_DEVICE_INTERFACE_LIST_PRESENT)
+	if err != nil {
+		return windows.InvalidHandle
+	}
+	var hubPath string
+	for _, candidate := range paths {
+		if candidate != "" {
+			hubPath = candidate
+			break
+		}
+	}
+	if hubPath == "" {
+		return windows.InvalidHandle
+	}
+	cached, found := p.hubs[hubPath]
+	if found {
+		return cached
+	}
+	handle := openHub(hubPath)
+	p.hubs[hubPath] = handle
+	return handle
+}
+
+func openHub(hubPath string) windows.Handle {
+	pathUTF16, err := windows.UTF16PtrFromString(hubPath)
+	if err != nil {
+		return windows.InvalidHandle
+	}
+	handle, err := windows.CreateFile(pathUTF16, windows.GENERIC_WRITE, windows.FILE_SHARE_WRITE, nil, windows.OPEN_EXISTING, 0, 0)
+	if err != nil {
+		return windows.InvalidHandle
+	}
+	return handle
+}
+
+func querySpeed(hub windows.Handle, port uint32) DeviceSpeed {
+	buffer := make([]byte, nodeConnInfoExBufferSize)
+	binary.LittleEndian.PutUint32(buffer[0:4], port)
+	var returned uint32
+	err := windows.DeviceIoControl(
+		hub,
+		ioctlUSBGetNodeConnectionInformationEx,
+		&buffer[0], uint32(len(buffer)),
+		&buffer[0], uint32(len(buffer)),
+		&returned, nil,
+	)
+	if err != nil || returned <= nodeConnInfoExSpeedOffset {
+		return SpeedUnknown
+	}
+	switch buffer[nodeConnInfoExSpeedOffset] {
+	case usbDeviceSpeedLow:
+		return SpeedLow
+	case usbDeviceSpeedFull:
+		return SpeedFull
+	case usbDeviceSpeedHigh:
+		return SpeedHigh
+	case usbDeviceSpeedSuper:
+		if superSpeedPlus(hub, port) {
+			return SpeedSuperPlus
+		}
+		return SpeedSuper
+	default:
+		return SpeedUnknown
+	}
+}
+
+func superSpeedPlus(hub windows.Handle, port uint32) bool {
+	var buffer [nodeConnInfoExV2Size]byte
+	binary.LittleEndian.PutUint32(buffer[0:4], port)
+	binary.LittleEndian.PutUint32(buffer[nodeConnInfoExV2LengthOff:], nodeConnInfoExV2Size)
+	var returned uint32
+	err := windows.DeviceIoControl(
+		hub,
+		ioctlUSBGetNodeConnectionInformationExV2,
+		&buffer[0], uint32(len(buffer)),
+		&buffer[0], uint32(len(buffer)),
+		&returned, nil,
+	)
+	if err != nil || returned < nodeConnInfoExV2Size {
+		return false
+	}
+	flags := binary.LittleEndian.Uint32(buffer[nodeConnInfoExV2FlagsOff:])
+	return flags&nodeConnInfoExV2FlagSuperSpeedPlus != 0
+}

service/usbip/data_protocol.go

@@ -294,6 +294,12 @@ func readUSBIPPayload(r io.Reader, direction uint32, bufferLength int32, packetC
 	if err != nil {
 		return nil, nil, err
 	}
+	if command {
+		err = validateUSBIPIsoDescriptorRanges(isoPackets, bufferLength)
+		if err != nil {
+			return nil, nil, err
+		}
+	}
 	return buffer, isoPackets, nil
 }
 
@@ -399,3 +405,21 @@ func validateUSBIPIsoPacketCount(count int32) error {
 	}
 	return nil
 }
+
+// validateUSBIPIsoDescriptorRanges rejects CMD_SUBMIT iso descriptors whose
+// offset/length fall outside the declared transfer buffer before they reach a
+// platform host engine. The IN copy-back path already clamps; the submit path
+// did not, leaving a remote peer's offsets unchecked against the buffer.
+func validateUSBIPIsoDescriptorRanges(packets []IsoPacketDescriptor, bufferLength int32) error {
+	for i := range packets {
+		offset := packets[i].Offset
+		length := packets[i].Length
+		if offset < 0 || length < 0 {
+			return E.New("USB/IP iso descriptor has negative offset/length: offset ", offset, ", length ", length)
+		}
+		if int64(offset)+int64(length) > int64(bufferLength) {
+			return E.New("USB/IP iso descriptor exceeds transfer buffer: offset ", offset, ", length ", length, ", buffer ", bufferLength)
+		}
+	}
+	return nil
+}

service/usbip/host_windows.go

@@ -95,8 +95,9 @@ func (h *windowsExportHost) Close() error {
 		_ = monitor.Close()
 	}
 	for _, exp := range exports {
-		if exp.device != nil {
-			_ = exp.device.Close()
+		device := exp.takeDevice()
+		if device != nil {
+			_ = device.Close()
 		}
 	}
 	return nil
@@ -191,9 +192,9 @@ func (h *windowsExportHost) FinishImport(busid string) (bool, error) {
 	if !ok {
 		return false, nil
 	}
-	if exp.device != nil {
-		_ = exp.device.Close()
-		exp.device = nil
+	device := exp.takeDevice()
+	if device != nil {
+		_ = device.Close()
 	}
 	return false, nil
 }
@@ -248,7 +249,27 @@ type windowsExport struct {
 	info   vboxusb.USBDeviceInfo
 	entry  DeviceEntry
 	logger log.ContextLogger
-	device *vboxusb.Device
+
+	deviceAccess sync.Mutex
+	device       *vboxusb.Device
+}
+
+// setDevice records the claimed handle once NewServerDataSession opens it.
+func (e *windowsExport) setDevice(device *vboxusb.Device) {
+	e.deviceAccess.Lock()
+	e.device = device
+	e.deviceAccess.Unlock()
+}
+
+// takeDevice atomically hands the claimed handle to exactly one caller and
+// clears the field, so FinishImport and Close racing on shutdown cannot both
+// close the same handle.
+func (e *windowsExport) takeDevice() *vboxusb.Device {
+	e.deviceAccess.Lock()
+	device := e.device
+	e.device = nil
+	e.deviceAccess.Unlock()
+	return device
 }
 
 func newWindowsExport(info vboxusb.USBDeviceInfo, logger log.ContextLogger) *windowsExport {
@@ -256,6 +277,7 @@ func newWindowsExport(info vboxusb.USBDeviceInfo, logger log.ContextLogger) *win
 		Info: DeviceInfoTruncated{
 			BusNum:             info.BusNumber,
 			DevNum:             info.Address,
+			Speed:              windowsSpeedToProtocol(info.Speed),
 			IDVendor:           info.VendorID,
 			IDProduct:          info.ProductID,
 			BCDDevice:          info.Revision,
@@ -268,6 +290,23 @@ func newWindowsExport(info vboxusb.USBDeviceInfo, logger log.ContextLogger) *win
 	return &windowsExport{info: info, entry: entry, logger: logger}
 }
 
+func windowsSpeedToProtocol(speed vboxusb.DeviceSpeed) uint32 {
+	switch speed {
+	case vboxusb.SpeedLow:
+		return SpeedLow
+	case vboxusb.SpeedFull:
+		return SpeedFull
+	case vboxusb.SpeedHigh:
+		return SpeedHigh
+	case vboxusb.SpeedSuper:
+		return SpeedSuper
+	case vboxusb.SpeedSuperPlus:
+		return SpeedSuperPlus
+	default:
+		return SpeedUnknown
+	}
+}
+
 func (e *windowsExport) BusID() string {
 	return e.info.BusID
 }
@@ -316,6 +355,6 @@ func (e *windowsExport) NewServerDataSession(ctx context.Context, conn net.Conn)
 		_ = device.Close()
 		return nil, E.New("windows usbip: device ", e.info.BusID, " is already claimed by another handle")
 	}
-	e.device = device
+	e.setDevice(device)
 	return newUserspaceURBSession(ctx, e.logger, conn, newVBoxUSBEngine(device)), nil
 }

service/usbip/protocol.go

@@ -6,6 +6,7 @@ import (
 	"encoding/binary"
 	"io"
 	"strings"
+	"unsafe"
 
 	E "github.com/sagernet/sing/common/exceptions"
 )
@@ -29,6 +30,20 @@ const (
 	deviceInterfaceWireSize  = 4
 )
 
+// DeviceInfoTruncated and DeviceInterface are serialized field-for-field
+// by binary.Write with no padding, so their in-memory size equals their
+// wire size. These two-sided constant assertions pin the hand-coded wire
+// bounds to the structs: a field added, removed, or resized changes the
+// kernel-visible layout and fails the build here instead of silently
+// mis-bounding the reader, whose only other safety net is the privileged
+// interop suite.
+const (
+	_ = uint(unsafe.Sizeof(DeviceInfoTruncated{})) - deviceInfoWireSize
+	_ = deviceInfoWireSize - uint(unsafe.Sizeof(DeviceInfoTruncated{}))
+	_ = uint(unsafe.Sizeof(DeviceInterface{})) - deviceInterfaceWireSize
+	_ = deviceInterfaceWireSize - uint(unsafe.Sizeof(DeviceInterface{}))
+)
+
 const (
 	SpeedUnknown   uint32 = 0
 	SpeedLow       uint32 = 1

service/usbip/server.go

@@ -199,6 +199,9 @@ func (s *ServerService) handleStandardConn(conn net.Conn, header OpHeader) {
 			s.logger.Debug("read import body: ", err)
 			break
 		}
+		// The connection becomes a data session below; drop the handshake
+		// read deadline so URB traffic is not bounded by it.
+		_ = conn.SetReadDeadline(time.Time{})
 		closeConn = !s.handleImportBusID(conn, busid)
 	default:
 		s.logger.Debug(fmt.Sprintf("unknown opcode 0x%04x", header.Code))
@@ -222,12 +225,17 @@ func (s *ServerService) handleControlConn(conn net.Conn) {
 		s.logger.Debug("unsupported control version ", hello.Version)
 		return
 	}
+	// The handshake read deadline from dispatchConn has served its purpose;
+	// readControlConn installs its own per-iteration idle deadline.
+	_ = conn.SetReadDeadline(time.Time{})
 	sub := s.ledger.Subscribe(conn)
 	defer s.ledger.Unsubscribe(sub)
+	_ = conn.SetWriteDeadline(time.Now().Add(controlWriteTimeout))
 	err = writeControlMessage(conn, controlFrame{
 		Type:    controlFrameAck,
 		Version: controlProtocolVersion,
 	}, nil)
+	_ = conn.SetWriteDeadline(time.Time{})
 	if err != nil {
 		s.logger.Debug("write control ack: ", err)
 		return
@@ -241,7 +249,9 @@ func (s *ServerService) handleControlConn(conn net.Conn) {
 		case <-readDone:
 			return
 		case message := <-sub.send:
+			_ = conn.SetWriteDeadline(time.Now().Add(controlWriteTimeout))
 			err = writeControlMessage(conn, message.Frame, message.Payload)
+			_ = conn.SetWriteDeadline(time.Time{})
 			if err != nil {
 				s.logger.Debug("write control frame: ", err)
 				return
@@ -267,7 +277,12 @@ func (s *ServerService) buildDevListEntries() []DeviceEntry {
 }
 
 func (s *ServerService) handleImportBusID(conn net.Conn, busid string) bool {
+	// Serialize the reservation against an in-flight Reconcile pass: both take
+	// reconcileAccess before inventoryAccess, so a reserve cannot interleave a
+	// pass that would otherwise release and close a just-reserved device.
+	s.reconcileAccess.Lock()
 	export, ok, reason := s.ledger.TryReserveForImport(busid)
+	s.reconcileAccess.Unlock()
 	if !ok {
 		s.logger.Info("import rejected (", busid, ": ", reason, ")")
 		_ = WriteOpRepImport(conn, OpRepImport, OpStatusError, nil)

service/usbip/server_shared.go

@@ -11,6 +11,11 @@ import (
 	E "github.com/sagernet/sing/common/exceptions"
 )
 
+// serverHandshakeTimeout bounds how long an unauthenticated peer may hold a
+// goroutine before sending its preface/hello. The post-handshake control loop
+// uses controlReadTimeout, which tolerates the client's controlPingInterval.
+const serverHandshakeTimeout = 10 * time.Second
+
 func (s *ServerService) acceptLoop(ln net.Listener) {
 	for {
 		conn, err := ln.Accept()
@@ -41,6 +46,7 @@ func (s *ServerService) acceptLoop(ln net.Listener) {
 func (s *ServerService) dispatchConn(conn net.Conn) {
 	cancelClose := closeConnOnContextDone(s.ctx, conn)
 	defer cancelClose()
+	_ = conn.SetReadDeadline(time.Now().Add(serverHandshakeTimeout))
 	var prefix [controlPrefaceSize]byte
 	_, err := io.ReadFull(conn, prefix[:])
 	if err != nil {
@@ -59,6 +65,10 @@ func (s *ServerService) readControlConn(sub *exportSubscriber, done chan<- struc
 	defer close(done)
 	var reader controlReader
 	for {
+		err := sub.conn.SetReadDeadline(time.Now().Add(controlReadTimeout))
+		if err != nil {
+			return
+		}
 		message, err := reader.read(sub.conn)
 		if err != nil {
 			return

service/usbip/usbhost_darwin.go

@@ -13,6 +13,7 @@ import "C"
 
 import (
 	"runtime/cgo"
+	"sync"
 	"unsafe"
 
 	E "github.com/sagernet/sing/common/exceptions"
@@ -110,8 +111,9 @@ type darwinUSBHostDeviceInfo struct {
 }
 
 type darwinUSBHostDevice struct {
-	handle *C.box_usbhost_device_t
-	info   darwinUSBHostDeviceInfo
+	handle    *C.box_usbhost_device_t
+	info      darwinUSBHostDeviceInfo
+	closeOnce sync.Once
 }
 
 func darwinCopyUSBHostDevices() ([]darwinUSBHostDeviceInfo, error) {
@@ -394,11 +396,13 @@ func box_usbip_darwin_usb_event(ref C.uintptr_t) {
 }
 
 func (d *darwinUSBHostDevice) Close() {
-	if d.handle == nil {
-		return
-	}
-	C.box_usbhost_device_close(d.handle)
-	d.handle = nil
+	d.closeOnce.Do(func() {
+		if d.handle == nil {
+			return
+		}
+		C.box_usbhost_device_close(d.handle)
+		d.handle = nil
+	})
 }
 
 func (d *darwinUSBHostDevice) control(setup [8]byte, buffer []byte) (int32, int32, []byte, error) {