search

PiSCSI / piscsi

PiSCSI allows a Raspberry Pi to function as emulated SCSI devices (hard disk, CD-ROM, and others) for vintage SCSI-based computers and devices. This is a fork of the RaSCSI project by GIMONS.
https://piscsi.org
BSD 3-Clause "New" or "Revised" License
537 stars 82 forks source link

SCSI VERIFY commands do not verify but write #807

Closed uweseimet closed 12 months ago

uweseimet commented 2 years ago

This affects release 22.08.01 and older.

In BYTCHK mode rascsi does not compare but writes the data sent for comparison. This is not considered a serious issue because clients that use the SCSI VERIFY commands can be expected to send the same data they sent with the preceding write. But nevertheless the current behavior is wrong. All VERIFY commands are optional, so maybe it is better to not support them at all.

uweseimet commented 2 years ago

@akuker @rdmark I tend to say we should remove the (broken) support for VERIFY. I doubt that any computer platform depends on these optional command sto be available.

akuker commented 2 years ago

I propose we keep it, but just have it return success? (it doesn't need to actually do anything)

uweseimet commented 2 years ago

@akuker I don't think we can do that. If we return success we have to accept the data sent by the initiator first. Directly going into the status phase instead of the data out phase and report success may cause issues. Drivers for the Atari ST platform, for instance, will quite likely run into issues, because the ACSI bus interface (where SCSI drives are usually connected to) is not very flexible as far as bus phases are concerned. As far as I can tell the problem with the existing code is that it cannot read data from the initiator without writing them to an image file. Just discarding these data is not possible, if my code analysis is correct. Can you double-check this?

uweseimet commented 2 years ago

@akuker @rdmark We need a decision here. IMO we have to remove VERIFY support, which is optional in the standard anyway. Would you buy a hard disk drive or SSD when you knew that some read operations actually write data? ;-) @akuker Can you please verify my error analysis? Just search for eCmdVerify to find the relevant code.

uweseimet commented 1 year ago

Dealing with eCmdVerify10 and eCmdVerify16 in a separate switch case, which is not calling disk->Write(), may be a solution. This would still not verify anything, but that's more acceptable than writing instead of actually verifying, because no data can be damaged anymore.

rdmark commented 12 months ago

@uweseimet Do we know why VERIFY was implemented like this in piscsi in the first place? (I tried to "git blame" myself to an answer, but couldn't find conclusive evidence.)

If it was a hack for a particular device or platform, we should note the breaking change in the upcoming changelog, IMHO.

uweseimet commented 12 months ago

@rdmark The device does not know what VERIFY is actually doing. From the device'perspective VERIFY just receives data, similar to WRITE. But VERIFY is a command that is not supposed to write any data, just to receive them. If (e.g. to a bug in a driver) the data transferred to VERIFY differ from the actual data already written, the existing data are corrupted. I guess the author of the original code just did not care and ignored the risks.