fix: address code review findings (nil handler, CmdSN, Data-Out order)

1. Discovery session nil handler crash: reject SCSI commands with
   Reject PDU when s.scsi is nil (discovery sessions have no target).

2. CmdSN window enforcement: validate incoming CmdSN against
   [ExpCmdSN, MaxCmdSN] using serial arithmetic. Drop out-of-window
   commands per RFC 7143 section 4.2.2.1.

3. Data-Out buffer offset validation: enforce BufferOffset == received
   for ordered data (DataPDUInOrder=Yes). Prevents silent corruption
   from out-of-order or overlapping data.

4. ImmediateData enforcement: reject immediate data in SCSI command
   PDU when negotiated ImmediateData=No.

5. UNMAP descriptor length alignment: reject blockDescLen not a
   multiple of 16 bytes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

weed/storage/blockvol/iscsi/dataio.go

@@ -6,9 +6,10 @@ import (
 )
 
 var (
-	ErrDataSNOrder    = errors.New("iscsi: Data-Out DataSN out of order")
-	ErrDataOverflow   = errors.New("iscsi: data exceeds expected transfer length")
-	ErrDataIncomplete = errors.New("iscsi: data transfer incomplete")
+	ErrDataSNOrder      = errors.New("iscsi: Data-Out DataSN out of order")
+	ErrDataOffsetOrder  = errors.New("iscsi: Data-Out buffer offset out of order")
+	ErrDataOverflow     = errors.New("iscsi: data exceeds expected transfer length")
+	ErrDataIncomplete   = errors.New("iscsi: data transfer incomplete")
 )
 
 // DataInWriter splits a read response into multiple Data-In PDUs, respecting
@@ -140,6 +141,11 @@ func (c *DataOutCollector) AddDataOut(pdu *PDU) error {
 		return ErrDataOverflow
 	}
 
+	// Validate buffer offset matches received position (DataPDUInOrder/DataSequenceInOrder)
+	if offset != c.received {
+		return ErrDataOffsetOrder
+	}
+
 	copy(c.buf[offset:], data)
 	c.received += uint32(len(data))
 

weed/storage/blockvol/iscsi/scsi.go

@@ -405,6 +405,9 @@ func (h *SCSIHandler) unmap(cdb [16]byte, dataOut []byte) SCSIResult {
 	if int(blockDescLen)+8 > len(dataOut) {
 		return illegalRequest(ASCInvalidFieldInCDB, ASCQLuk)
 	}
+	if blockDescLen%16 != 0 {
+		return illegalRequest(ASCInvalidFieldInCDB, ASCQLuk)
+	}
 
 	// Each UNMAP block descriptor is 16 bytes
 	descData := dataOut[8 : 8+blockDescLen]

weed/storage/blockvol/iscsi/session.go

@@ -45,6 +45,10 @@ type Session struct {
 	negotiator *LoginNegotiator
 	loginDone  bool
 
+	// Negotiated session parameters
+	negImmediateData bool
+	negInitialR2T    bool
+
 	// Data sequencing
 	dataInWriter *DataInWriter
 
@@ -177,8 +181,12 @@ func (s *Session) handleLogin(pdu *PDU) error {
 		s.state = SessionLoggedIn
 		result := s.negotiator.Result()
 		s.dataInWriter = NewDataInWriter(uint32(result.MaxRecvDataSegLen))
+		s.negImmediateData = result.ImmediateData
+		s.negInitialR2T = result.InitialR2T
 
-		// Bind SCSI handler to the device for the target the initiator logged into
+		// Bind SCSI handler to the device for the target the initiator logged into.
+		// Discovery sessions have no TargetName — s.scsi stays nil, which is
+		// checked in handleSCSICmd.
 		if s.devices != nil && result.TargetName != "" {
 			dev := s.devices.LookupDevice(result.TargetName)
 			s.scsi = NewSCSIHandler(dev)
@@ -215,12 +223,25 @@ func (s *Session) handleSCSICmd(pdu *PDU) error {
 		return s.sendReject(pdu, 0x0b) // protocol error
 	}
 
+	// Discovery sessions have no SCSI handler — reject commands
+	if s.scsi == nil {
+		return s.sendReject(pdu, 0x04) // command not supported
+	}
+
 	cdb := pdu.CDB()
 	itt := pdu.InitiatorTaskTag()
 	flags := pdu.OpSpecific1()
 
-	// Advance CmdSN
+	// CmdSN validation for non-immediate commands (RFC 7143 section 4.2.2.1)
 	if !pdu.Immediate() {
+		cmdSN := pdu.CmdSN()
+		expCmdSN := s.expCmdSN.Load()
+		maxCmdSN := s.maxCmdSN.Load()
+		// CmdSN is within window if ExpCmdSN <= CmdSN <= MaxCmdSN (serial arithmetic)
+		if !cmdSNInWindow(cmdSN, expCmdSN, maxCmdSN) {
+			s.logger.Printf("CmdSN %d out of window [%d, %d], dropping", cmdSN, expCmdSN, maxCmdSN)
+			return nil // silently drop per RFC 7143
+		}
 		s.advanceCmdSN()
 	}
 
@@ -233,8 +254,12 @@ func (s *Session) handleSCSICmd(pdu *PDU) error {
 	if isWrite && expectedLen > 0 {
 		collector := NewDataOutCollector(expectedLen)
 
-		// Immediate data
+		// Immediate data — enforce negotiated ImmediateData flag
 		if len(pdu.DataSegment) > 0 {
+			if !s.negImmediateData {
+				// ImmediateData=No but initiator sent data — reject
+				return s.sendCheckCondition(itt, SenseIllegalRequest, ASCInvalidFieldInCDB, ASCQLuk)
+			}
 			if err := collector.AddImmediateData(pdu.DataSegment); err != nil {
 				return s.sendCheckCondition(itt, SenseIllegalRequest, ASCInvalidFieldInCDB, ASCQLuk)
 			}
@@ -388,6 +413,17 @@ func (s *Session) advanceCmdSN() {
 	s.maxCmdSN.Add(1)
 }
 
+// cmdSNInWindow checks if cmdSN is within [expCmdSN, maxCmdSN] using
+// serial number arithmetic (RFC 7143 section 4.2.2.1). Handles uint32 wrap.
+func cmdSNInWindow(cmdSN, expCmdSN, maxCmdSN uint32) bool {
+	// Serial comparison: a <= b means (b - a) < 2^31
+	return serialLE(expCmdSN, cmdSN) && serialLE(cmdSN, maxCmdSN)
+}
+
+func serialLE(a, b uint32) bool {
+	return a == b || int32(b-a) > 0
+}
+
 func (s *Session) sendReject(origPDU *PDU, reason uint8) error {
 	resp := &PDU{}
 	resp.SetOpcode(OpReject)