mirror of
https://github.com/akuker/RASCSI.git
synced 2024-12-11 02:49:28 +00:00
Multiple fixes for ModeSelect (#1405)
* Allow 'empty' ModeSelect6 tl;dr Treat a computed length of 0 as `has_valid_page_code`. Details: The SRM console (aka 'BIOS') of DEC Alpha sends an empty ModeSelect6 with the following data: ~~~ ModeSelect6, CDB $151000000c00 ~~~ That makes 12 byte(s) as follows ~~~ 0 1 2 3 4 5 6 7 8 9 10 11 00 00 00 08 00 00 00 00 00 00 02 00 ~~~ decoding it (accoring to [1], Section 8.3.3, Table 94) gives us Mode Data Length 0 Medium Type 0 Device-specific 0 Block desc len 8 Density Code 0 Number of blks 0 Reserved 0 Block length 512 `scsi_command_util::ModeSelect` computes ~~~ offset = 4 + buf[3]; ~~~ giving 12 and ~~~ length -= offset; ~~~ giving 0. Thus it never enters the `while` loop and `has_valid_page_code` stays `false`, raising an error. [1] [Small Computer System Interface - 2 rev 10L.pdf](https://dn790004.ca.archive.org/0/items/SCSISpecificationDocumentsSCSIDocuments/Small%20Computer%20System%20Interface%20-%202%20rev%2010L.pdf) Signed-off-by: Klaus Kämpf <kkaempf@gmail.com> * Allow ModeSelect with page code 1 OpenVMS Alpha (the operating system, not the SRM BIOS) uses ModeSelect6 with a page code of 1. The semantics are unknown, just accepting it works for me. Signed-off-by: Klaus Kämpf <kkaempf@gmail.com> * Fix page length computation in ModeSelect tl;dr The 'skip to next ModeSelect page' computation was off-by-one, either not taking the page code itself into account or missing the fact that the page length is given as `n - 1`. Fix: Add 1 to the computed length. Details: OpenVMS Alpha sends a ModeSelect6 as follows ~~~ command: ModeSelect6, CDB $151000001900 payload: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 00 00 00 08 00 00 00 00 00 00 02 00 01 0a 24 00 00 00 00 00 00 00 00 00 00 ~~~ This translates to (accoring to [1], Section 8.3.3) ~~~ Mode Data Length 0 Medium Type 0 Device-specific 0 Block desc len 8 ~~~ with the following offset / length computation _before_ the `while` loop ~~~ offset = 12 length = 13 ~~~ The first payload section is ~~~ 4 5 6 7 8 9 10 11 00 00 00 00 00 00 02 00 ~~~ translating to ~~~ Density Code 0 Number of blks 0 Reserved 0 Block length 0x200 512 ~~~ Then follows a pagecode 1 as ~~~ 12 13 14 15 16 17 18 19 20 21 22 23 24 01 0a 24 00 00 00 00 00 00 00 00 00 00 ~~~ translating to ~~~~ Page code 1 Page length -1 10 Mode parameters 24 00 00 00 00 00 00 00 00 00 00 ~~~ computing (inside the `while` loop, as `// Advance to the next page`) ~~~ size = 10 + 2 = 12 ~~~ followed by new `offset` and `length` values ~~~ offset = 25 length = 1 ~~~ So it stays in the `while` loop (and has a larger-than-buffer `offset` value) Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
This commit is contained in:
parent
256650908b
commit
ad5eae93e7
@ -40,7 +40,8 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
|
||||
}
|
||||
length -= offset;
|
||||
|
||||
bool has_valid_page_code = false;
|
||||
// treat zero length as valid
|
||||
bool has_valid_page_code = (length == 0);
|
||||
|
||||
// Parse the pages
|
||||
while (length > 0) {
|
||||
@ -62,6 +63,10 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
|
||||
|
||||
has_valid_page_code = true;
|
||||
}
|
||||
else if (page == 0x01) {
|
||||
// OpenVMS Alpha 7.3 uses this
|
||||
has_valid_page_code = true;
|
||||
}
|
||||
else {
|
||||
stringstream s;
|
||||
s << "Unknown MODE SELECT page code: $" << setfill('0') << setw(2) << hex << page;
|
||||
@ -71,7 +76,7 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
|
||||
// Advance to the next page
|
||||
const int size = buf[offset + 1] + 2;
|
||||
|
||||
length -= size;
|
||||
length -= size + 1;
|
||||
offset += size;
|
||||
}
|
||||
|
||||
|
@ -46,6 +46,11 @@ TEST(ScsiCommandUtilTest, ModeSelect6)
|
||||
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
|
||||
<< "Unsupported page 0 was not rejected";
|
||||
|
||||
// Page 1
|
||||
buf[12] = 0x01;
|
||||
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512))
|
||||
<< "Page 1 is supported";
|
||||
|
||||
// Page 3 (Format Device Page)
|
||||
buf[12] = 0x03;
|
||||
EXPECT_THAT([&] { ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512); },
|
||||
@ -62,7 +67,25 @@ TEST(ScsiCommandUtilTest, ModeSelect6)
|
||||
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
|
||||
<< "Not enough command parameters";
|
||||
|
||||
EXPECT_FALSE(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512).empty());
|
||||
// check length computation
|
||||
buf[3] = 8;
|
||||
buf[10] = 2;
|
||||
buf[12] = 1;
|
||||
buf[13] = 10;
|
||||
buf[14] = 0x24;
|
||||
buf[24] = 0;
|
||||
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, LENGTH, 512))
|
||||
<< "Multi-page length computation";
|
||||
|
||||
// check length computation
|
||||
buf[3] = 8;
|
||||
buf[10] = 12;
|
||||
buf[12] = 0;
|
||||
buf[13] = 0;
|
||||
buf[14] = 0;
|
||||
buf[24] = 0;
|
||||
EXPECT_NO_THROW(ModeSelect(scsi_command::eCmdModeSelect6, cdb, buf, 12, 512))
|
||||
<< "Empty ModeSelect6";
|
||||
}
|
||||
|
||||
TEST(ScsiCommandUtilTest, ModeSelect10)
|
||||
@ -111,8 +134,6 @@ TEST(ScsiCommandUtilTest, ModeSelect10)
|
||||
Property(&scsi_exception::get_sense_key, sense_key::illegal_request),
|
||||
Property(&scsi_exception::get_asc, asc::invalid_field_in_parameter_list))))
|
||||
<< "Not enough command parameters";
|
||||
|
||||
EXPECT_FALSE(ModeSelect(scsi_command::eCmdModeSelect10, cdb, buf, LENGTH, 512).empty());
|
||||
}
|
||||
|
||||
TEST(ScsiCommandUtilTest, EnrichFormatPage)
|
||||
|
Loading…
Reference in New Issue
Block a user