Repository navigation
SCSI tape: report an I/O error as MEDIUM ERROR - #646
Open
LispEngineer wants to merge 5 commits into
Open
LispEngineer wants to merge 5 commits into
LispEngineer wants to merge 5 commits into
Conversation
The disk read handlers pass the command's sector count straight to
sim_disk_rdsect(), which reads that many sectors into bus->buf, and the
write handlers set buf_b from the command and then fill the buffer to it.
Nothing checks the count against the size of that buffer, which each
controller chooses when it calls scsi_init(): 64KB on the NCR 53C94 of
vax4xx_rz94.c (VAXstation 4000-60 and VLC, MicroVAX 3100-80, InfoServer
1000) and on the 3B2's host adapter; on the NCR 5380 of vax4xx_rz80.c, 16KB
on the MicroVAX 2000 and 128KB on the KA420 and KA43 models. Read(6) allows
256 sectors -- 128KB -- and Read(10) and Write(10) allow 65535. On an
unattached unit the read handlers memset() the same length. scsi_read()
and scsi_data() then walk the buffer bounded only by buf_b, never by the
allocation, so an oversized transfer reads or writes past the end of
bus->buf.
Reject such a command with CHECK CONDITION (ILLEGAL REQUEST / INVALID FIELD
IN CDB), which is what a host adapter with a maximum transfer size does;
clamping the count instead would report success for a transfer shorter than
the one requested. scsi_init() already knows the size -- it allocates the
buffer -- so record it in the bus, and also clamp buf_b to it in the two
transfer loops, so that no caller can make them leave their buffer.
The transfer length need not come from the command. When sim_disk_rdsect()
fails it can return without setting its sector count, and the read
handlers use that count as the length without checking the status (a
separate change makes them check it). That is reachable from the console
of a vaxstation4000m60, with no guest and any file as the disk:
$ truncate -s 4M blank.img
sim> set cpu 80m
sim> set rom nodelay
sim> attach rz0 blank.img
sim> set rz0 format=auto
sim> boot cpu
>>> boot dka0
SET RZ0 FORMAT=AUTO on an attached unit leaves its container format
unresolved, so every read fails in sim_disk_rdsect() with the count unset.
Without this change the first Read(10) goes to DATA IN with whatever the
uninitialised count held -- 11,184,640 bytes, read under a debugger,
against the 65,536 byte buffer -- and the simulator dies with SIGSEGV in
scsi_read(). (That is with XS left unattached, as above. With ATTACH XS
NAT: the memory behind the buffer happened to be mapped, and the guest was
handed whatever lay there instead: BOOT DKA0 printed "?43 FILESTRUCT".)
With this change the loop stays inside the buffer, the simulator survives,
and the boot reports "?43 FILESTRUCT" from the data it was given; the
status change is what stops that.
OpenVMS does not reach the command-time limit: the largest read it issues
on this controller is 127 sectors, one below it. It is reachable by any
initiator that asks for more, and the reproduction above does not reach it
(the command asks for one block), so it was tested by rewriting the first
Read(10) that the console ROM issues on BOOT DKA0 under a debugger to ask
for 129 sectors: without this change the handler goes to DATA IN with a
66048 byte transfer; with it, it answers CHECK CONDITION.
Tested on Open SIMH 87eb7d5 (vaxstation4000m60): the reproduction above
dies with SIGSEGV in scsi_read() in 5 runs of 5 without this change, and
survives with it. vaxstation4000m60, microvax3100m80, vaxstation4000vlc,
microvax3900, microvax2000 and 3b2 build and pass their register sanity
check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tape read and write handlers take the length of a transfer from the command, 24 bits and so up to 16 MB, and nothing compares it with the bus transfer buffer (64 KB on the 53C94, 16 KB on the MicroVAX 2000's 5380): - a QIC read of fixed blocks reads Transfer Length blocks of 512 bytes, one after another into the buffer, without a limit: 200 blocks write 100 KB; - a streaming read passes the transfer length (times the block size if FIXED) to sim_tape_rdrecf() as the largest record it may put in the buffer, so a 100 KB record overruns it; - a write sets buf_b from the command and then calls sim_tape_wrrecf() with that many bytes, of which the buffer holds buf_size. A fixed-block read, and a write, that the buffer cannot hold are refused at the command with CHECK CONDITION, ILLEGAL REQUEST, INVALID FIELD IN CDB, as scsi_xfer_too_big() does for disks. A variable length read is not refused: its transfer length is a limit on the record, not the length of the transfer, and a shorter record is read as it is, so only the library's limit is clamped to the buffer; a record the buffer cannot hold comes back as an overlength record and nothing is written past the end. A write of zero bytes or blocks writes nothing and is GOOD at the command, with no DATA OUT phase. Depends on scsi-transfer-bounds-v2 (open-simh/simh open-simh#590) for bus->buf_size and scsi_xfer_too_big(); it is cut on that branch. Tested with tests/scsi_paths.py (cases tagged tapebounds), which drives a SCSI initiator on the bus under gdb with a canary after the buffer. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- sim_tape_rdrecf() stores the record length through its count before it reads the record and returns an I/O error after it; scsi_read6_tape() used the count as the length of the transfer, so a failed read went to DATA IN with whatever the buffer held, then CHECK CONDITION with sense key NO SENSE. An overlength record (MTSE_INVRL) is the same: nothing was read and the count is the record's length. Only a record that was read (MTSE_OK, or MTSE_RECE for a record with its error flag) is data now. - An overlength record is CHECK CONDITION with ILI and no data, in the fixed and variable modes alike. With SILI set and a block length of zero it is not an error (SCSI-2, READ command): the first Transfer Length bytes come back with GOOD status and the tape moves past the record, which the handler used to do with the contents of the buffer. - scsi_tape_status() had no default: MTSE_RUNAWAY, and anything it does not list, left the status GOOD; MTSE_IOERR was CHECK CONDITION with sense key NO SENSE. An I/O error or anything unlisted is HARDWARE ERROR now, as a failed disk read is since open-simh#589; MTSE_LEOT (logical end of tape, data written) is CHECK CONDITION with EOM and NO SENSE, a drive's early warning; MTSE_INVRL carries ILI; MTSE_WRP has additional sense code WRITE PROTECTED; MTSE_UNATT is NOT READY, MEDIUM NOT PRESENT as TEST UNIT READY says it, not BLANK CHECK. - A tape read or write with no tape loaded was GOOD: the read returned a zero-filled buffer, the write discarded its data. Both are NOT READY, MEDIUM NOT PRESENT. - scsi_wrfmark() passed an uninitialised status to scsi_tape_status() when the count was zero, which is legal. Depends on the previous commit and on scsi-transfer-bounds-v2; KEY_HWERR is the one open-simh#589 adds and ASC_WRTPROT the one open-simh#591 adds, so the #define block has an adjacent-line conflict with them, resolved by keeping both sides. Tested with tests/scsi_paths.py (cases tagged tapestatus). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A READ of a tape record shorter than the requested length, with SILI clear, sets the deferred CHECK CONDITION with the incorrect length indicator and the residue (requested less actual, SCSI-2 10.2.4) -- and then the status that every read ends with, scsi_tape_status (bus, r), replaced it with GOOD, so the initiator got the short record, GOOD status and no ILI. Remember the residue where it is found and set the deferred status and the information field after the common status has been set. The record is still returned as data, followed by CHECK CONDITION with ILI, NO SENSE and the residue in the information bytes, which is how a drive answers. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This change depends on open-simh#639 ("SCSI tape: keep transfers inside the bus buffer and report I/O status", branch scsi-tape-io-status) and is to be merged after it: it changes the default case of the scsi_tape_status() that pull request makes report HARDWARE ERROR. It adds the KEY_MEDERR, ASC_UNRERR and ASC_WRTERR names that open-simh#589 and open-simh#637 also add, with the same values and spelling. A tape unit is a container file, and the file is the medium. When reading or writing it fails (MTSE_IOERR), the drive has not failed as a device: the data on the medium could not be read or written. SCSI-2 8.2.14.3 defines MEDIUM ERROR as a non-recovered error probably caused by a flaw in the medium and pairs it with UNRECOVERED READ ERROR (11h) and WRITE ERROR (0Ch); open-simh#637 makes the same change for the disk handlers. scsi_tape_status() serves reads, writes and the positioning commands, so it cannot choose between 11h and 0Ch. The new scsi_tape_io_status() calls it and replaces its answer for MTSE_IOERR (HARDWARE ERROR) with MEDIUM ERROR and the additional sense code the caller gives. The other statuses that fall into that default case, such as MTSE_RUNAWAY, are not changed. READ(6), in its streaming and its QIC loop, uses ASC_UNRERR; WRITE(6) uses ASC_WRTERR. The other callers (ERASE, WRITE FILEMARKS, SPACE, REWIND) keep HARDWARE ERROR. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This applies on top of #639 (
scsi-tape-io-status): the branch shows #639'sthree commits below this one until #639 is merged.
#639 made the tape handlers report the status of
sim_tape_rdrecf()andsim_tape_wrrecf(); an I/O error (MTSE_IOERR) is CHECK CONDITION with sensekey HARDWARE ERROR and no additional sense code. #637 ("SCSI: report a failed
host read or write as MEDIUM ERROR") changes the disk handlers from HARDWARE
ERROR to MEDIUM ERROR, with UNRECOVERED READ ERROR (11h) for a read and WRITE
ERROR (0Ch) for a write. This does the same for the tape.
A tape unit here is a container file, and the file is the medium. When reading
or writing it fails, the drive has not failed as a device: the data on the
medium could not be read or written. SCSI-2 (X3.131-1994, 8.2.14.3) pairs
MEDIUM ERROR with those additional sense codes and reserves HARDWARE ERROR for
a failure of the target.
scsi_tape_status()is used for reads, writes and the positioning commands,so it cannot choose between 11h and 0Ch. A new
scsi_tape_io_status (bus, status, asc)calls it and replaces its answer forMTSE_IOERRwith MEDIUMERROR and the additional sense code its caller gives. The two READ(6) sites
(the streaming read, and the loop of the QIC fixed-block read) pass 11h; the
WRITE(6) site passes 0Ch. Only
MTSE_IOERRchanges: the other statuses thatfall into
scsi_tape_status()'s default case, such asMTSE_RUNAWAY, stayHARDWARE ERROR, and so do the callers for ERASE, WRITE FILEMARKS, SPACE and
REWIND (a positioning or filemark failure has no pair of codes to choose).
Why the key matters to a guest is measured on #637's text: OpenVMS reads a
block that reports MEDIUM ERROR a few times and returns a status for it, and
treats HARDWARE ERROR as the drive failing, with repeated retries and mount
verification. (That is measured for disks; it was not measured for a tape.)
How to reproduce
vaxstation4000m60, no guest, a tape file attached to aTZ30orTZK50, anda SCSI initiator on
rz_busunder gdb withsim_tape_rdrecf()orsim_tape_wrrecf()made to returnMTSE_IOERRafter the real call (as thetests do): READ(6) is CHECK CONDITION with sense key 4 and no additional
sense code on #639, key 3 with 11h on this branch; WRITE(6) key 4 / 00h and
key 3 / 0Ch. The QIC drive's fixed-block read (a
TZK50, FIXED bit set) takesits own loop and is the third case.
Testing
All fourteen simulators that link
sim_scsi.cbuild with the makefile's-Werrorand pass their register sanity check, on this branch alone and on atrial merge with the rest of the fork's
integrationbranch. The three cases taggedtapemediumfail on #639 (HARDWARE ERROR, no additional sense code) and passhere; the 20 cases tagged
tapeboundsandtapestatusof #639 still pass(
tape-read-runawaystill asserts HARDWARE ERROR;tape-read-ioerroracceptseither key, so that it holds before and after this change).
🤖 Generated with Claude Code