wangjia/sing-box
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

usbip: validate ret_submit iso descriptors and lock handoff Close fields

Browse Source 
P2: Darwin IN iso responses accepted malformed RET_SUBMIT descriptors —
aggregate ActualLength was checked but per-descriptor Offset/Length were
not, and ScatterIsoResponse silently clamped bad values into apparent
success. ValidateIsoResponse now lives in iso_scheduler.go beside
EncodeIsoSubmit/RebaseFrame and enforces the single-packet shape (Offset
== 0, Length == requestLen, ActualLength <= Length, sum == header,
payload covers range). pendingTransfer.validateResponse routes every
RET_SUBMIT shape check through one named seam so future defects land in
one place, and ScatterIsoResponse drops its defensive clamps so the
data-movement primitive can no longer mask a validation gap.

P2: kernelHandoffSession.Close mutated h.conn/h.monitorFile/h.relayConn
outside stateAccess while Start reads them under that lock; the server
registers prepared sessions before Start, so Close-before-Start can race
the direct-TCP path during import-time shutdown. Close now snapshots and
nils the three fields under stateAccess before closing the locals via
closeOnce, restoring symmetry with Start's under-lock reads and matching
the copy-under-lock, close-outside idiom established by
exportLedger.CloseAllSubscribers and ServerService.Close.
This commit is contained in:
世界
2026-05-16 15:24:10 +08:00
parent 4db2957a49
commit e0ad90bcd9
3 changed files with 88 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
service/usbip/endpoint_darwin.go
+35 -16
View File
@@ -207,33 +207,52 @@ func (e *darwinEndpoint) finalizePending(pending *pendingTransfer) {
}
}
// accept validates a RET_SUBMIT against the original request and, for IN
// transfers, scatters the payload into the Apple-owned buffer. The returned
// length is guaranteed to be in [0, p.requestLen] regardless of direction.
// A non-nil err signals a protocol violation; the caller is expected to
// cancel the endpoint so no further wire-corrupt completions are delivered
// to IOUSBHost.
func (p *pendingTransfer) accept(response SubmitResponse) (int32, int, error) {
// validateResponse reconciles a RET_SUBMIT against the original request shape.
// Every wire-shape rule lives here so accept can assume well-formed input and
// future defects land in one place. Returns the wire-level errno status and a
// non-nil err on protocol violation.
func (p *pendingTransfer) validateResponse(response SubmitResponse) (int32, error) {
if response.ActualLength < 0 {
return -int32(unix.EPROTO), 0, errResponseNegativeActualLength
return -int32(unix.EPROTO), errResponseNegativeActualLength
}
if int(response.ActualLength) > p.requestLen {
return -int32(unix.EOVERFLOW), errResponseOverflow
}
if p.direction != USBIPDirIn {
return 0, nil
}
if len(response.Buffer) > p.requestLen {
return -int32(unix.EOVERFLOW), errResponseOverflow
}
if len(response.IsoPackets) > 0 {
err := ValidateIsoResponse(p.requestLen, int(response.ActualLength), response.IsoPackets, len(response.Buffer))
if err != nil {
return -int32(unix.EPROTO), err
}
}
return 0, nil
}
// accept validates a RET_SUBMIT against the original request and, for IN
// transfers, scatters the payload into the Apple-owned buffer. A non-nil err
// signals a protocol violation; the caller is expected to cancel the endpoint
// so no further wire-corrupt completions are delivered to IOUSBHost.
func (p *pendingTransfer) accept(response SubmitResponse) (int32, int, error) {
errStatus, err := p.validateResponse(response)
if err != nil {
return errStatus, 0, err
}
actualLength := int(response.ActualLength)
if actualLength > p.requestLen {
return -int32(unix.EOVERFLOW), 0, errResponseOverflow
}
if p.direction != USBIPDirIn {
return response.Status, actualLength, nil
}
if len(response.Buffer) > p.requestLen {
return -int32(unix.EOVERFLOW), 0, errResponseOverflow
}
copyLength := min(actualLength, len(response.Buffer))
if copyLength > 0 && p.bufferPtr != nil {
dst := unsafe.Slice((*byte)(p.bufferPtr), p.requestLen)
if len(response.IsoPackets) > 0 {
dst := unsafe.Slice((*byte)(p.bufferPtr), p.requestLen)
ScatterIsoResponse(dst, response.Buffer[:copyLength], response.IsoPackets)
} else {
copy(unsafe.Slice((*byte)(p.bufferPtr), copyLength), response.Buffer[:copyLength])
copy(dst[:copyLength], response.Buffer[:copyLength])
}
}
return response.Status, actualLength, nil
service/usbip/handoff_linux.go
+10 -6
View File
@@ -108,17 +108,21 @@ func (h *kernelHandoffSession) Err() error {
func (h *kernelHandoffSession) Close() error {
h.stateAccess.Lock()
h.closed = true
conn := h.conn
monitorFile := h.monitorFile
relayConn := h.relayConn
h.conn = nil
h.monitorFile = nil
h.relayConn = nil
h.stateAccess.Unlock()
h.closeOnce.Do(func() {
h.closeErr = E.Errors(
h.closeKernelFD(),
common.Close(h.monitorFile),
common.Close(h.relayConn),
common.Close(h.conn),
common.Close(monitorFile),
common.Close(relayConn),
common.Close(conn),
)
h.monitorFile = nil
h.relayConn = nil
h.conn = nil
})
h.markDone(nil)
return h.closeErr
service/usbip/iso_scheduler.go
+43 -15
View File
@@ -2,6 +2,10 @@
package usbip
import (
E "github.com/sagernet/sing/common/exceptions"
)
// EncodeIsoSubmit fills the isochronous SUBMIT fields on base. When asap is
// true, the wire-level ASAP flag is set and StartFrame is zeroed. Otherwise
// RebaseFrame recovers the absolute frame number from the controller's
@@ -31,26 +35,50 @@ func RebaseFrame(currentFrame uint64, low8 uint8) uint64 {
return base
}
var (
errIsoDescriptorCount = E.New("RET_SUBMIT iso descriptor count mismatch")
errIsoDescriptorRange = E.New("RET_SUBMIT iso descriptor range mismatch")
errIsoDescriptorActualLength = E.New("RET_SUBMIT iso descriptor actual_length exceeds length")
errIsoDescriptorSum = E.New("RET_SUBMIT iso descriptor actual_length sum does not match header")
errIsoPayloadShort = E.New("RET_SUBMIT iso payload shorter than descriptor range")
)
// ValidateIsoResponse enforces the per-descriptor invariants of a RET_SUBMIT
// against the original CMD_SUBMIT shape. startIsoTransfer emits single-packet
// CMD_SUBMITs that cover the whole request, so the response must mirror that
// exact shape. If multi-packet ISO submits are added later, the count check
// relaxes to a non-empty count and the offset/length check becomes a per-
// descriptor walk that proves coverage is non-overlapping and in-range.
func ValidateIsoResponse(requestLen int, actualLength int, packets []IsoPacketDescriptor, payloadLen int) error {
if len(packets) != 1 {
return E.Extend(errIsoDescriptorCount, "expected 1, got ", len(packets))
}
descriptor := packets[0]
if descriptor.Offset != 0 || int(descriptor.Length) != requestLen {
return E.Extend(errIsoDescriptorRange, "offset ", descriptor.Offset, ", length ", descriptor.Length, ", request ", requestLen)
}
if descriptor.ActualLength < 0 || descriptor.ActualLength > descriptor.Length {
return E.Extend(errIsoDescriptorActualLength, "actual_length ", descriptor.ActualLength, ", length ", descriptor.Length)
}
if int(descriptor.ActualLength) != actualLength {
return E.Extend(errIsoDescriptorSum, "sum ", descriptor.ActualLength, " != header ", actualLength)
}
if int(descriptor.ActualLength) > payloadLen {
return E.Extend(errIsoPayloadShort, "actual_length ", descriptor.ActualLength, " > payload ", payloadLen)
}
return nil
}
// ScatterIsoResponse copies frame data from payload into dst at the offsets
// declared by packets. Caller MUST have called ValidateIsoResponse against the
// same descriptors and payload before invoking; this function trusts every
// offset and length and will panic on slice bounds for malformed input.
func ScatterIsoResponse(dst, payload []byte, packets []IsoPacketDescriptor) {
cursor := 0
for i := range packets {
length := int(packets[i].ActualLength)
if length <= 0 {
continue
}
if cursor+length > len(payload) {
length = len(payload) - cursor
if length <= 0 {
return
}
}
offset := int(packets[i].Offset)
if offset < 0 || offset >= len(dst) {
cursor += length
continue
}
end := min(offset+length, len(dst))
copy(dst[offset:end], payload[cursor:cursor+(end-offset)])
copy(dst[offset:offset+length], payload[cursor:cursor+length])
cursor += length
}
}