Fix length calculation in scsi_command_util::ModeSelect

OpenVMS Alpha sends a strange ModeSelect payload, apparently one byte
too large. This was 'fixed' by a (wrong) length calculation in #1405, breaking #1427.

This PR
- fixes the wrong length calculation
- improves the loop test in scsi_command_util::ModeSelect to prevent a
  buffer overflow. (Remaining length was checked for > 0, but buffer
  access is at offset and offset + 1, effectively requiring 2 bytes.)
- the loop test fix makes #1402 pass
- adds a testcase for #1402
- adds a testcase for #1427

Fixes issue #1427

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
This commit is contained in:
Klaus Kämpf 2024-04-01 15:47:55 +02:00
parent b1a3ffbc7d
commit 73917bce17
3 changed files with 58 additions and 2 deletions

View File

@ -44,7 +44,8 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span<const uin
bool has_valid_page_code = (length == 0);
// Parse the pages
while (length > 0) {
// expect (remaining) length > 1 because we access buf[offset+1] below
while (length > 1) {
// Format device page
if (const int page = buf[offset]; page == 0x03) {
if (length < 14) {
@ -76,7 +77,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 + 1;
length -= size;
offset += size;
}

View File

@ -363,6 +363,9 @@ class MockSCSIHD : public SCSIHD //NOSONAR Ignore inheritance hierarchy depth in
FRIEND_TEST(ScsiHdTest, DECSpecialFunctionControlPage);
FRIEND_TEST(ScsiHdTest, GetSectorSizes);
FRIEND_TEST(ScsiHdTest, ModeSelect);
FRIEND_TEST(ScsiHdTest, PageCode1);
FRIEND_TEST(ScsiHdTest, MultiplePages);
FRIEND_TEST(PiscsiExecutorTest, SetSectorSize);
public:

View File

@ -140,3 +140,55 @@ TEST(ScsiHdTest, ModeSelect)
buf[20] = 0x02;
EXPECT_NO_THROW(hd.ModeSelect(scsi_command::eCmdModeSelect10, cmd, buf, 255)) << "MODE SELECT(10) is supported";
}
// Test for the ModeSelect6, page code 1 (issued by Alpha VMS)
TEST(ScsiHdTest, PageCode1)
{
MockSCSIHD hd(0, false);
vector<int> cmd = { 0x15, 0x10, 0x00, 0x00, 0x19, 0x00 };
vector<uint8_t> buf = { 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00
, 0x01, 0x0a, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
EXPECT_NO_THROW(hd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, buf.size())) << "Page code 1 is supported";
}
// Test for the ModeSelect6, multiple pages
TEST(ScsiHdTest, MultiplePages)
{
MockSCSIHD hd(0, false);
vector<uint8_t> buf = { 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00
, 0x02, 0x01, 0x00 // 12
, 0x02, 0x01, 0x00 // 15
, 0x02, 0x01, 0x00 // 18
, 0x02, 0x01, 0x00 // 21
, 0x02, 0x01, 0x00 // 24
, 0x02, 0x01, 0x00 // 27
, 0x02, 0x01, 0x00 // 30
, 0x02, 0x01, 0x00 // 33
, 0x02, 0x01, 0x00 // 36
, 0x02, 0x01, 0x00 // 39
, 0x02, 0x01, 0x00 // 42
, 0x02, 0x01, 0x00 // 45
, 0x02, 0x01, 0x00 // 48
, 0x02, 0x01, 0x00 // 51
, 0x02, 0x01, 0x00 // 54
, 0x02, 0x01, 0x00 // 57
, 0x02, 0x01, 0x00 // 60
, 0x02, 0x01, 0x00 // 63
, 0x02, 0x01, 0x00 // 66
, 0x02, 0x01, 0x00 // 69
, 0x02, 0x01, 0x00 // 72
, 0x02, 0x01, 0x00 // 75
, 0x02, 0x01, 0x00 // 78
, 0x02, 0x01, 0x00 // 81
, 0x02, 0x01, 0x00 // 84
, 0x02, 0x01, 0x00 // 87
, 0x02, 0x01, 0x00 // 90
, 0x03, 0x16, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x19, 0x08, 0x00, 0x00, 0x01, 0x00, 0x0b, 0x00, 0x14, 0x00, 0x00, 0x00, 0x00
};
vector<int> cmd = { 0x15, 0x10, 0x00, 0x00, static_cast<int>(buf.size()), 0x00 };
hd.SetSectorSizeInBytes(2048); // pass the "page 3" sector_size test in scsi_command_util::ModeSelect
EXPECT_NO_THROW(hd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, buf.size())) << "Multiple pages are supported";
}