From 905ab8384650ebb40dacde68246756235db5d8c8 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 3 Dec 2025 09:50:03 -0500 Subject: [PATCH 1/3] fix: separate host expected bytes from device intended bytes --- src/machine/usb/msc/msc.go | 21 +++++++++++---------- src/machine/usb/msc/scsi.go | 26 +++++++++++++++----------- src/machine/usb/msc/scsi_inquiry.go | 4 ++-- src/machine/usb/msc/scsi_readwrite.go | 10 +++++----- src/machine/usb/msc/scsi_unmap.go | 2 +- 5 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/machine/usb/msc/msc.go b/src/machine/usb/msc/msc.go index d3bf8d6e29..420a06ed98 100644 --- a/src/machine/usb/msc/msc.go +++ b/src/machine/usb/msc/msc.go @@ -35,12 +35,12 @@ type msc struct { respStatus csw.Status // Response status for the last command sendZLP bool // Flag to indicate if a zero-length packet should be sent before sending CSW - cbw *CBW // Last received Command Block Wrapper - queuedBytes uint32 // Number of bytes queued for sending - sentBytes uint32 // Number of bytes sent - totalBytes uint32 // Total bytes to send - cswBuf []byte // CSW response buffer - state mscState + cbw *CBW // Last received Command Block Wrapper + queuedBytes uint32 // Number of bytes queued for sending + sentBytes uint32 // Number of bytes sent + transferBytes uint32 // Total bytes to send + cswBuf []byte // CSW response buffer + state mscState maxLUN uint8 // Maximum Logical Unit Number (n-1 for n LUNs) dev machine.BlockDevice @@ -160,8 +160,9 @@ func (m *msc) sendUSBPacket(b []byte) { func (m *msc) sendCSW(status csw.Status) { // Generate CSW packet into m.cswBuf and send it residue := uint32(0) - if m.totalBytes >= m.sentBytes { - residue = m.totalBytes - m.sentBytes + expected := m.cbw.transferLength() + if expected >= m.sentBytes { + residue = expected - m.sentBytes } m.cbw.CSW(status, residue, m.cswBuf) m.state = mscStateStatusSent @@ -245,7 +246,7 @@ func (m *msc) run(b []byte, isEpOut bool) bool { // Move on to the data transfer phase next go around (after sending the first message) m.state = mscStateData - m.totalBytes = cbw.transferLength() + m.transferBytes = cbw.transferLength() m.queuedBytes = 0 m.sentBytes = 0 m.respStatus = csw.StatusPassed @@ -281,7 +282,7 @@ func (m *msc) run(b []byte, isEpOut bool) bool { // to cycle back through this block, e.g. with TEST UNIT READY which sends only a CSW after // setting the sense key/add'l code/qualifier internally if m.state == mscStateStatus && !m.txStalled { - if m.totalBytes > m.sentBytes && m.cbw.isIn() { + if m.cbw.transferLength() > m.sentBytes && m.cbw.isIn() { // 6.7.2 The Thirteen Cases - Case 5 (Hi > Di): STALL before status m.stallEndpoint(usb.MSC_ENDPOINT_IN) } else if m.sendZLP { diff --git a/src/machine/usb/msc/scsi.go b/src/machine/usb/msc/scsi.go index d7266ed40f..4babd5d5bd 100644 --- a/src/machine/usb/msc/scsi.go +++ b/src/machine/usb/msc/scsi.go @@ -21,9 +21,9 @@ func (m *msc) scsiCmdBegin() { return } - if m.totalBytes > 0 && m.cbw.isOut() { + if m.transferBytes > 0 && m.cbw.isOut() { // Reject any other multi-packet commands - if m.totalBytes > m.maxPacketSize { + if m.transferBytes > m.maxPacketSize { m.sendScsiError(csw.StatusFailed, scsi.SenseIllegalRequest, scsi.SenseCodeInvalidCmdOpCode) return } else { @@ -53,7 +53,7 @@ func (m *msc) scsiCmdBegin() { } if len(m.buf) == 0 { - if m.totalBytes > 0 { + if m.transferBytes > 0 { // 6.7.2 The Thirteen Cases - Case 4 (Hi > Dn) // https://usb.org/sites/default/files/usbmassbulk_10.pdf m.sendScsiError(csw.StatusFailed, scsi.SenseIllegalRequest, 0) @@ -63,7 +63,7 @@ func (m *msc) scsiCmdBegin() { m.state = mscStateStatus } } else { - if m.totalBytes == 0 { + if m.transferBytes == 0 { // 6.7.1 The Thirteen Cases - Case 2 (Hn < Di) // https://usb.org/sites/default/files/usbmassbulk_10.pdf m.sendScsiError(csw.StatusFailed, scsi.SenseIllegalRequest, 0) @@ -93,7 +93,7 @@ func (m *msc) scsiDataTransfer(b []byte) bool { // Update our sent bytes count to include the just-confirmed bytes m.sentBytes += m.queuedBytes - if m.sentBytes >= m.totalBytes { + if m.sentBytes >= m.transferBytes { // Transfer complete, send CSW after transfer confirmed m.state = mscStateStatus } else if cmdType == scsi.CmdRead { @@ -158,8 +158,8 @@ func (m *msc) scsiCmdModeSense(cmd scsi.Cmd) { // The host allows a good amount of leeway in response size // Reset total bytes to what we'll actually send - if m.totalBytes > respLen { - m.totalBytes = respLen + if m.transferBytes > respLen { + m.transferBytes = respLen m.sendZLP = true } @@ -210,7 +210,7 @@ func (m *msc) scsiCmdRequestSense() { // Set the buffer size to the SCSI sense message size and clear m.resetBuffer(scsi.RequestSenseRespLen) m.queuedBytes = scsi.RequestSenseRespLen - m.totalBytes = scsi.RequestSenseRespLen + m.transferBytes = scsi.RequestSenseRespLen // 0x70 - current error, 0x71 - deferred error (not used) m.buf[0] = 0xF0 // 0x70 for current error plus 0x80 for valid flag bit @@ -266,7 +266,7 @@ func (m *msc) scsiQueueTask(cmdType scsi.CmdType, b []byte) bool { switch cmdType { case scsi.CmdWrite: // If we're writing data wait until we have a full write block of data that can be processed. - if m.queuedBytes == uint32(cap(m.blockCache)) { + if m.queuedBytes == uint32(cap(m.blockCache)) || (m.sentBytes+m.queuedBytes >= m.transferBytes) { m.taskQueued = true } case scsi.CmdUnmap: @@ -279,7 +279,11 @@ func (m *msc) scsiQueueTask(cmdType scsi.CmdType, b []byte) bool { func (m *msc) sendScsiError(status csw.Status, key scsi.Sense, code scsi.SenseCode) { // Generate CSW into m.cswBuf - residue := m.totalBytes - m.sentBytes + expected := m.cbw.transferLength() + residue := uint32(0) + if expected > m.sentBytes { + residue = expected - m.sentBytes + } // Prepare to send CSW m.sendZLP = true // Ensure the transaction is signaled as ended before a CSW is sent @@ -291,7 +295,7 @@ func (m *msc) sendScsiError(status csw.Status, key scsi.Sense, code scsi.SenseCo m.addlSenseCode = code m.addlSenseQualifier = 0x00 // Not used - if m.totalBytes > 0 && residue > 0 { + if expected > 0 && residue > 0 { if m.cbw.isIn() { m.stallEndpoint(usb.MSC_ENDPOINT_IN) } else { diff --git a/src/machine/usb/msc/scsi_inquiry.go b/src/machine/usb/msc/scsi_inquiry.go index ae7028f63f..4c1c583cb1 100644 --- a/src/machine/usb/msc/scsi_inquiry.go +++ b/src/machine/usb/msc/scsi_inquiry.go @@ -136,13 +136,13 @@ func (m *msc) scsiEvpdInquiry(cmd scsi.Cmd, pageCode uint8) { // Set total bytes to the length of our response m.queuedBytes = uint32(len(m.buf)) - m.totalBytes = uint32(len(m.buf)) + m.transferBytes = uint32(len(m.buf)) } func (m *msc) scsiStdInquiry(cmd scsi.Cmd) { m.resetBuffer(scsi.InquiryRespLen) m.queuedBytes = scsi.InquiryRespLen - m.totalBytes = scsi.InquiryRespLen + m.transferBytes = scsi.InquiryRespLen // byte 0 - Device Type (0x00 for direct access block device) // byte 1 - Removable media bit diff --git a/src/machine/usb/msc/scsi_readwrite.go b/src/machine/usb/msc/scsi_readwrite.go index 1b09e13418..8693566196 100644 --- a/src/machine/usb/msc/scsi_readwrite.go +++ b/src/machine/usb/msc/scsi_readwrite.go @@ -12,7 +12,7 @@ func (m *msc) scsiCmdReadWrite(cmd scsi.Cmd) { status := m.validateScsiReadWrite(cmd) if status != csw.StatusPassed { m.sendScsiError(status, scsi.SenseIllegalRequest, scsi.SenseCodeInvalidCmdOpCode) - } else if m.totalBytes > 0 { + } else if m.transferBytes > 0 { if cmd.CmdType() == scsi.CmdRead { m.scsiRead(cmd) } else { @@ -28,7 +28,7 @@ func (m *msc) scsiCmdReadWrite(cmd scsi.Cmd) { func (m *msc) validateScsiReadWrite(cmd scsi.Cmd) csw.Status { blockCount := cmd.BlockCount() // CBW wrapper transfer length - if m.totalBytes == 0 { + if m.transferBytes == 0 { // If the SCSI command's block count doesn't loosely match the wrapper's transfer length something's wrong if blockCount > 0 { return csw.StatusPhaseError @@ -50,7 +50,7 @@ func (m *msc) validateScsiReadWrite(cmd scsi.Cmd) csw.Status { // https://usb.org/sites/default/files/usbmassbulk_10.pdf return csw.StatusFailed } - if m.totalBytes/blockCount == 0 { + if m.transferBytes/blockCount == 0 { // Block size shouldn't be small enough to round to zero // 6.7.2 The Thirteen Cases - Case 7 (Hi < Di) READ(10) or // 6.7.3 The Thirteen Cases - Case 13 (Ho < Do) WRITE(10) @@ -103,7 +103,7 @@ func (m *msc) writeBlock(b []byte, lba, offset uint32) (n int, err error) { func (m *msc) scsiRead(cmd scsi.Cmd) { // Make sure we don't exceed the buffer size - readEnd := m.totalBytes - m.sentBytes + readEnd := m.transferBytes - m.sentBytes if readEnd > m.maxPacketSize { readEnd = m.maxPacketSize } @@ -136,7 +136,7 @@ func (m *msc) scsiWrite(cmd scsi.Cmd, b []byte) { m.sentBytes += uint32(len(b)) } - if m.sentBytes >= m.totalBytes { + if m.sentBytes >= m.transferBytes { // Data transfer is complete, send CSW m.state = mscStateStatus m.run([]byte{}, true) diff --git a/src/machine/usb/msc/scsi_unmap.go b/src/machine/usb/msc/scsi_unmap.go index 79c2426ddc..1d09ec8ed3 100644 --- a/src/machine/usb/msc/scsi_unmap.go +++ b/src/machine/usb/msc/scsi_unmap.go @@ -60,7 +60,7 @@ func (m *msc) scsiUnmap(b []byte) { // FIXME: We need to handle erase block alignment m.sentBytes += uint32(len(b)) - if m.sentBytes >= m.totalBytes { + if m.sentBytes >= m.transferBytes { // Order 66 complete, send CSW to establish galactic empire m.state = mscStateStatus m.run([]byte{}, true) From 25be55fd801f94a87a4353bf4c8a85821904e4b0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 3 Dec 2025 09:55:31 -0500 Subject: [PATCH 2/3] fix: update m.queuedBytes when clamping output to avoid corrupting sentBytes --- src/machine/usb/msc/scsi.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/machine/usb/msc/scsi.go b/src/machine/usb/msc/scsi.go index 4babd5d5bd..4cec23e2f2 100644 --- a/src/machine/usb/msc/scsi.go +++ b/src/machine/usb/msc/scsi.go @@ -72,6 +72,7 @@ func (m *msc) scsiCmdBegin() { if m.cbw.transferLength() < uint32(len(m.buf)) { m.buf = m.buf[:m.cbw.transferLength()] } + m.queuedBytes = uint32(len(m.buf)) m.sendUSBPacket(m.buf) } } From ce8fca4f895204b1e8d9aeb57e9b594e666f842b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 4 Dec 2025 18:05:37 -0500 Subject: [PATCH 3/3] fix: don't hardcode success return state fix: validate bmRequestType\nfix: mimic tinyusb behavior in mscStateNeedReset\nfix: CLEAR_FEATURE requests to iface addr are against spec --- src/machine/usb/msc/setup.go | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/machine/usb/msc/setup.go b/src/machine/usb/msc/setup.go index 3d5bef2d5a..00507aac69 100644 --- a/src/machine/usb/msc/setup.go +++ b/src/machine/usb/msc/setup.go @@ -3,7 +3,6 @@ package msc import ( "machine" "machine/usb" - "machine/usb/msc/csw" ) func setupPacketHandler(setup usb.Setup) bool { @@ -18,11 +17,17 @@ func (m *msc) setupPacketHandler(setup usb.Setup) bool { wValue := (uint16(setup.WValueH) << 8) | uint16(setup.WValueL) switch setup.BRequest { case usb.CLEAR_FEATURE: - ok = m.handleClearFeature(setup, wValue) + if setup.BmRequestType == 0x02 { // Host-to-Device | Standard | Endpoint + ok = m.handleClearFeature(setup, wValue) + } case usb.GET_MAX_LUN: - ok = m.handleGetMaxLun(setup, wValue) + if setup.BmRequestType == 0xA1 { // Device-to-Host | Class | Interface + ok = m.handleGetMaxLun(setup, wValue) + } case usb.MSC_RESET: - ok = m.handleReset(setup, wValue) + if setup.BmRequestType == 0x21 { // Host-to-Device | Class | Interface + ok = m.handleReset(setup, wValue) + } } return ok } @@ -53,24 +58,25 @@ func (m *msc) handleClearFeature(setup usb.Setup, wValue uint16) bool { } else if wIndex == usb.MSC_ENDPOINT_OUT { m.stallEndpoint(usb.MSC_ENDPOINT_OUT) } - return ok + machine.SendZlp() + return true } // Clear the direction bit from the endpoint address for comparison wIndex := setup.WIndex & 0x7F - // Clear the IN/OUT stalls if addressed to the endpoint, or both if addressed to the interface - if wIndex == usb.MSC_ENDPOINT_IN || wIndex == mscInterface { + // Clear the IN/OUT stalls if addressed to the endpoint + if wIndex == usb.MSC_ENDPOINT_IN { m.clearStallEndpoint(usb.MSC_ENDPOINT_IN) ok = true } - if wIndex == usb.MSC_ENDPOINT_OUT || wIndex == mscInterface { + if wIndex == usb.MSC_ENDPOINT_OUT { m.clearStallEndpoint(usb.MSC_ENDPOINT_OUT) ok = true } // Send a CSW if needed to resume after the IN endpoint stall is cleared if m.state == mscStateStatus && wIndex == usb.MSC_ENDPOINT_IN { - m.sendCSW(csw.StatusPassed) + m.sendCSW(m.respStatus) ok = true }