diff --git a/common/vboxusb/pnp_windows.go b/common/vboxusb/pnp_windows.go index 0f44bffe7..7238bee7a 100644 --- a/common/vboxusb/pnp_windows.go +++ b/common/vboxusb/pnp_windows.go @@ -41,6 +41,7 @@ type USBDeviceInfo struct { Address uint32 // device address on the bus (port path leaf) BusID string // "-
" 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 diff --git a/common/vboxusb/speed_windows.go b/common/vboxusb/speed_windows.go new file mode 100644 index 000000000..e8184aaf6 --- /dev/null +++ b/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 +} diff --git a/service/usbip/data_protocol.go b/service/usbip/data_protocol.go index c9903b6f8..0c7cab76d 100644 --- a/service/usbip/data_protocol.go +++ b/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 +} diff --git a/service/usbip/host_windows.go b/service/usbip/host_windows.go index d1cd89a7e..4be0f2a7c 100644 --- a/service/usbip/host_windows.go +++ b/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 } diff --git a/service/usbip/protocol.go b/service/usbip/protocol.go index 8f00901f9..8f0a6ecb0 100644 --- a/service/usbip/protocol.go +++ b/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 diff --git a/service/usbip/server.go b/service/usbip/server.go index c67f73529..037e1798b 100644 --- a/service/usbip/server.go +++ b/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) diff --git a/service/usbip/server_shared.go b/service/usbip/server_shared.go index 2d55e1c60..d04f3be8a 100644 --- a/service/usbip/server_shared.go +++ b/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 diff --git a/service/usbip/usbhost_darwin.go b/service/usbip/usbhost_darwin.go index 6ace7ad01..5cd63ae18 100644 --- a/service/usbip/usbhost_darwin.go +++ b/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) {