From 0eae853c5f15bbc15389c68f37e72ea4aa3a0ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Fri, 15 May 2026 19:20:02 +0800 Subject: [PATCH] usbip: unify linux teardown and handoff lifecycle --- service/usbip/handoff_linux.go | 51 ++++++++++++++++--------- service/usbip/host_linux.go | 38 ++++++++++--------- service/usbip/linux_test.go | 68 ++++++++++++++++++++++++++++++++++ service/usbip/server.go | 2 + 4 files changed, 124 insertions(+), 35 deletions(-) diff --git a/service/usbip/handoff_linux.go b/service/usbip/handoff_linux.go index f91dbec53..bd8fbe186 100644 --- a/service/usbip/handoff_linux.go +++ b/service/usbip/handoff_linux.go @@ -33,9 +33,12 @@ type kernelHandoffSession struct { done chan struct{} doneOnce sync.Once runErr error - startOnce sync.Once closeOnce sync.Once closeErr error + + stateAccess sync.Mutex + started bool + closed bool } func newKernelHandoffSession(ctx context.Context, conn net.Conn, logger log.ContextLogger, side string, busid string) (*kernelHandoffSession, error) { @@ -103,6 +106,9 @@ func (h *kernelHandoffSession) Err() error { } func (h *kernelHandoffSession) Close() error { + h.stateAccess.Lock() + h.closed = true + h.stateAccess.Unlock() h.closeOnce.Do(func() { h.closeErr = E.Errors( h.closeKernelFD(), @@ -126,22 +132,31 @@ func (h *kernelHandoffSession) markDone(err error) { } func (h *kernelHandoffSession) Start() error { - h.startOnce.Do(func() { - if h.relayConn == nil { - err := h.conn.Close() - if err != nil && !E.IsClosedOrCanceled(err) { - h.logger.Debug("close usbip ", h.side, " userspace socket ", h.busid, ": ", err) - } - h.conn = nil - monitorFile := h.monitorFile - h.monitorFile = nil - go h.runDirect(h.ctx, h.logger, h.side, h.busid, monitorFile) - return + h.stateAccess.Lock() + if h.started || h.closed { + h.stateAccess.Unlock() + return nil + } + h.started = true + conn := h.conn + relayConn := h.relayConn + monitorFile := h.monitorFile + h.stateAccess.Unlock() + + if relayConn == nil { + err := common.Close(conn) + if err != nil && !E.IsClosedOrCanceled(err) { + h.logger.Debug("close usbip ", h.side, " userspace socket ", h.busid, ": ", err) } - relayConn := h.relayConn - h.relayConn = nil - go h.runRelay(h.ctx, h.logger, h.side, h.busid, relayConn) - }) + h.stateAccess.Lock() + if h.conn == conn { + h.conn = nil + } + h.stateAccess.Unlock() + go h.runDirect(h.ctx, h.logger, h.side, h.busid, monitorFile) + return nil + } + go h.runRelay(h.ctx, h.logger, h.side, h.busid, conn, relayConn) return nil } @@ -182,8 +197,8 @@ func (h *kernelHandoffSession) runDirect(ctx context.Context, logger log.Context } } -func (h *kernelHandoffSession) runRelay(ctx context.Context, logger log.ContextLogger, side string, busid string, relayConn net.Conn) { - err := sBufio.CopyConn(ctx, h.conn, relayConn) +func (h *kernelHandoffSession) runRelay(ctx context.Context, logger log.ContextLogger, side string, busid string, conn net.Conn, relayConn net.Conn) { + err := sBufio.CopyConn(ctx, conn, relayConn) var runErr error switch { case err == nil: diff --git a/service/usbip/host_linux.go b/service/usbip/host_linux.go index 1fcfedf60..901ee32ff 100644 --- a/service/usbip/host_linux.go +++ b/service/usbip/host_linux.go @@ -168,7 +168,7 @@ func (h *linuxExportHost) Close() error { h.exports = make(map[string]*linuxExport) h.access.Unlock() for _, exp := range exports { - releaseErr := h.releaseExport(exp, exp.shouldRestoreCurrentDevice()) + releaseErr := h.releaseExport(exp) if releaseErr != nil { h.logger.Warn("rollback ", exp.busid, ": ", releaseErr) } @@ -338,7 +338,7 @@ func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid strin } for _, exp := range plan.toRelease { - releaseErr := h.releaseExport(exp, false) + releaseErr := h.releaseExport(exp) if releaseErr != nil { h.logger.Warn("release ", exp.busid, ": ", releaseErr) reconcileErrors = append(reconcileErrors, E.Cause(releaseErr, "release ", exp.busid)) @@ -411,7 +411,7 @@ func (h *linuxExportHost) FinishImport(ctx context.Context, busid string) (bool, if !ok || !exp.stale { return false, nil } - releaseErr := h.releaseExport(exp, false) + releaseErr := h.releaseExport(exp) h.access.Lock() current, stillPresent := h.exports[busid] if stillPresent && current == exp { @@ -569,7 +569,7 @@ func (h *linuxExportHost) bindOneOnce(d *sysfsDevice) (*linuxExport, error) { return h.newExport(*d, true, driver), nil } -func (h *linuxExportHost) releaseExport(exp *linuxExport, restore bool) error { +func (h *linuxExportHost) releaseExport(exp *linuxExport) error { if !exp.managed { h.logger.Info("stopped tracking ", exp.busid, " on usbip-host") return nil @@ -585,14 +585,18 @@ func (h *linuxExportHost) releaseExport(exp *linuxExport, restore bool) error { } } err := writeSysfs(filepath.Join(sysUsbipHostDriver, "unbind"), exp.busid) - if err != nil && !os.IsNotExist(err) && !(isMissingUSBDeviceError(err) && !restore) { + if err != nil && !os.IsNotExist(err) && !isMissingUSBDeviceError(err) { return err } err = writeSysfs(filepath.Join(sysUsbipHostDriver, "match_busid"), "del "+exp.busid) if err != nil { return err } - if !restore { + restoreCurrentDevice, err := h.shouldRestoreCurrentDevice(exp) + if err != nil { + return err + } + if !restoreCurrentDevice { h.logger.Info("removed export state for ", exp.busid) return nil } @@ -608,6 +612,17 @@ func (h *linuxExportHost) releaseExport(exp *linuxExport, restore bool) error { return nil } +func (h *linuxExportHost) shouldRestoreCurrentDevice(exp *linuxExport) (bool, error) { + descriptor, err := readSysfsDevice(exp.busid, filepath.Join(sysBusUSBDevices, exp.busid)) + if err != nil { + if os.IsNotExist(err) || isMissingUSBDeviceError(err) { + return false, nil + } + return false, E.Cause(err, "read current device ", exp.busid) + } + return exp.identity.Equal(newLinuxExportIdentity(descriptor)), nil +} + func (h *linuxExportHost) newExport(descriptor sysfsDevice, managed bool, originalDriver string) *linuxExport { return &linuxExport{ busid: descriptor.BusID, @@ -734,17 +749,6 @@ func (e *linuxExport) NewServerDataSession(ctx context.Context, conn net.Conn) ( return handoff, nil } -func (e *linuxExport) shouldRestoreCurrentDevice() bool { - if e.originalDriver == "" || e.stale { - return false - } - descriptor, err := readSysfsDevice(e.busid, filepath.Join(sysBusUSBDevices, e.busid)) - if err != nil { - return false - } - return e.identity.Equal(newLinuxExportIdentity(descriptor)) -} - type linuxImportHost struct { logger log.ContextLogger diff --git a/service/usbip/linux_test.go b/service/usbip/linux_test.go index 888248535..9b9e9a915 100644 --- a/service/usbip/linux_test.go +++ b/service/usbip/linux_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/sagernet/sing-box/log" + "github.com/sagernet/sing-box/option" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" @@ -294,6 +295,42 @@ func TestUSBIPConnHandoffDirectTCP(t *testing.T) { } } +func TestUSBIPConnHandoffCloseBeforeStartIsSafe(t *testing.T) { + t.Parallel() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + + accepted := make(chan net.Conn, 1) + go func() { + conn, _ := listener.Accept() + accepted <- conn + }() + + conn, err := net.Dial("tcp", listener.Addr().String()) + require.NoError(t, err) + acceptedConn := <-accepted + defer acceptedConn.Close() + + handoff, err := newKernelHandoffSession(context.Background(), conn, newTestLogger(t), "test", "close-before-start") + require.NoError(t, err) + + require.NoError(t, handoff.Close()) + require.NoError(t, handoff.Start()) + + select { + case <-handoff.Done(): + default: + t.Fatal("expected close-before-start handoff to be done") + } + + setConnDeadline(t, acceptedConn) + buffer := make([]byte, 1) + _, err = acceptedConn.Read(buffer) + require.ErrorIs(t, err, io.EOF) +} + func TestUSBIPConnHandoffRelaySocketpairCopies(t *testing.T) { t.Parallel() @@ -367,3 +404,34 @@ func TestUSBIPLinuxSmoke(t *testing.T) { require.NoError(t, err) require.Equal(t, "usb", driver) } + +func TestUSBIPLinuxReconcileReleaseRestoresOriginalDriver(t *testing.T) { + requireRoot(t) + + requireUSBIPHost(t) + requireVHCI(t) + + gadget := newTestUSBGadget(t) + host := newLinuxExportHost(newTestLogger(t), []option.USBIPDeviceMatch{{BusID: gadget.busid}}) + require.NoError(t, host.Start(context.Background())) + + snapshot, released, err := host.Reconcile(context.Background(), func(string) bool { return false }) + require.NoError(t, err) + require.Empty(t, released) + _, exported := snapshot[gadget.busid] + require.True(t, exported) + + driver, err := currentDriver(gadget.busid) + require.NoError(t, err) + require.Equal(t, "usbip-host", driver) + + host.matches = nil + snapshot, released, err = host.Reconcile(context.Background(), func(string) bool { return false }) + require.NoError(t, err) + require.Equal(t, []string{gadget.busid}, released) + require.Empty(t, snapshot) + + driver, err = currentDriver(gadget.busid) + require.NoError(t, err) + require.Equal(t, "usb", driver) +} diff --git a/service/usbip/server.go b/service/usbip/server.go index 79be54c8f..26473bd60 100644 --- a/service/usbip/server.go +++ b/service/usbip/server.go @@ -331,6 +331,8 @@ func (s *ServerService) handleImportReserved(conn net.Conn, busid string, export s.tearDownPreparedSession(busid, session) return false } + // Close may observe a prepared session before Start runs, so + // DataSession implementations must treat Close-before-Start as valid. s.sessions[session] = struct{}{} s.sessionsWG.Add(1) s.sessionsAccess.Unlock()