Possible handle leak: RemoveUSBBus returns before deleting its device handles when RemoveBus fails
RemoveUSBBus releases the cgo.Handles it holds for a bus only after
hw.s.RemoveBus has succeeded.
lib/viiper/bus.go:62
if err := hw.s.RemoveBus(busID); err != nil {
return false
}
hw.mtx.Lock()
defer hw.mtx.Unlock()
for _, dh := range hw.deviceHandles[busID] {
cgo.Handle(dh).Delete()
}
delete(hw.deviceHandles, busID)
The server removes buses on its own, so the failing case is reachable in normal
operation. RemoveBus returns bus %d not found for a bus already dropped
(internal/server/usb/server.go:232), and RemoveDeviceByID drops the bus once it
goes empty, from its cleanup goroutine at internal/server/usb/server.go:275. The
HTTP disconnect timeout takes devices straight off the bus
(internal/server/api/server.go:299) without touching hw.deviceHandles, which is
how the library ends up holding handles for a bus the server no longer has.
Each retained deviceHandle pins a deviceHandleWrapper (lib/viiper/viiper.go:33),
which holds usbServer, so the whole usbServerHandleWrapper graph stays
reachable along with it.
CloseUSBServer in the same package gets the ordering right: it deletes every
device handle (lib/viiper/server.go:121) before it checks hw.s.Close() for an
error (server.go:129).
Suggested fix: run the delete loop and the delete(hw.deviceHandles, busID)
before the RemoveBus error check, or on both paths, and keep returning false
for the caller. The bus is being torn down either way, so the handles are not
needed after this call.
If you could credit me as a reporter for my contributions to security advisory I will be thankful.
Possible handle leak: RemoveUSBBus returns before deleting its device handles when RemoveBus fails
RemoveUSBBusreleases thecgo.Handles it holds for a bus only afterhw.s.RemoveBushas succeeded.lib/viiper/bus.go:62
The server removes buses on its own, so the failing case is reachable in normal
operation.
RemoveBusreturnsbus %d not foundfor a bus already dropped(internal/server/usb/server.go:232), and
RemoveDeviceByIDdrops the bus once itgoes empty, from its cleanup goroutine at internal/server/usb/server.go:275. The
HTTP disconnect timeout takes devices straight off the bus
(internal/server/api/server.go:299) without touching
hw.deviceHandles, which ishow the library ends up holding handles for a bus the server no longer has.
Each retained
deviceHandlepins adeviceHandleWrapper(lib/viiper/viiper.go:33),which holds
usbServer, so the wholeusbServerHandleWrappergraph staysreachable along with it.
CloseUSBServerin the same package gets the ordering right: it deletes everydevice handle (lib/viiper/server.go:121) before it checks
hw.s.Close()for anerror (server.go:129).
Suggested fix: run the delete loop and the
delete(hw.deviceHandles, busID)before the
RemoveBuserror check, or on both paths, and keep returningfalsefor the caller. The bus is being torn down either way, so the handles are not
needed after this call.
If you could credit me as a reporter for my contributions to security advisory I will be thankful.