Fix status code handling, reject subpage requests

uweseimet · Oct 26, 2024 · 5ec9d7f · 5ec9d7f
1 parent 1036a6c
commit 5ec9d7f
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 5 deletions.
diff --git a/cpp/controllers/controller.cpp b/cpp/controllers/controller.cpp
@@ -202,7 +202,7 @@ void Controller::Status()
 
     // If this is a successfully terminated linked command convert the status code
     GetBuffer()[0] =
-        linked && GetStatus() == status_code::good ? (uint8_t)status_code::intermediate : (uint8_t)status_code::good;
+        linked && GetStatus() == status_code::good ? (uint8_t)status_code::intermediate : (uint8_t)GetStatus();
 }
 
 void Controller::MsgIn()

diff --git a/cpp/devices/host_services.cpp b/cpp/devices/host_services.cpp
@@ -211,8 +211,8 @@ void HostServices::ReceiveOperationResults()
 
 int HostServices::ModeSense6(cdb_t cdb, vector<uint8_t> &buf) const
 {
-    // Block descriptors cannot be returned
-    if (!(cdb[1] & 0x08)) {
+    // Block descriptors cannot be returned, subpages are not supported
+    if (cdb[3] || !(cdb[1] & 0x08)) {
         throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_cdb);
     }
 
@@ -229,8 +229,8 @@ int HostServices::ModeSense6(cdb_t cdb, vector<uint8_t> &buf) const
 
 int HostServices::ModeSense10(cdb_t cdb, vector<uint8_t> &buf) const
 {
-    // Block descriptors cannot be returned
-    if (!(cdb[1] & 0x08)) {
+    // Block descriptors cannot be returned, subpages are not supported
+    if (cdb[3] || !(cdb[1] & 0x08)) {
         throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_cdb);
     }
 

diff --git a/cpp/devices/storage_device.cpp b/cpp/devices/storage_device.cpp
@@ -375,6 +375,11 @@ off_t StorageDevice::GetFileSize() const
 
 int StorageDevice::ModeSense6(cdb_t cdb, vector<uint8_t> &buf) const
 {
+    // Subpages are not supported
+    if (cdb[3]) {
+        throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_cdb);
+    }
+
     const auto length = static_cast<int>(min(buf.size(), static_cast<size_t>(cdb[4])));
     fill_n(buf.begin(), length, 0);
 
@@ -417,6 +422,11 @@ int StorageDevice::ModeSense6(cdb_t cdb, vector<uint8_t> &buf) const
 
 int StorageDevice::ModeSense10(cdb_t cdb, vector<uint8_t> &buf) const
 {
+    // Subpages are not supported
+    if (cdb[3]) {
+        throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_cdb);
+    }
+
     const auto length = static_cast<int>(min(buf.size(), static_cast<size_t>(GetInt16(cdb, 7))));
     fill_n(buf.begin(), length, 0);
 

diff --git a/cpp/test/host_services_test.cpp b/cpp/test/host_services_test.cpp
@@ -143,6 +143,9 @@ TEST(HostServicesTest, ModeSense6)
     EXPECT_NO_THROW(services->Dispatch(scsi_command::mode_sense6));
     buffer = controller->GetBuffer();
     EXPECT_EQ(0x01, buffer[0]);
+
+    controller->SetCdbByte(3, 0x01);
+    EXPECT_THROW(services->Dispatch(scsi_command::mode_sense6), scsi_exception)<< "Subpages are not supported";
 }
 
 TEST(HostServicesTest, ModeSense10)
@@ -177,6 +180,9 @@ TEST(HostServicesTest, ModeSense10)
     EXPECT_NO_THROW(services->Dispatch(scsi_command::mode_sense10));
     buffer = controller->GetBuffer();
     EXPECT_EQ(0x02, buffer[1]);
+
+    controller->SetCdbByte(3, 0x01);
+    EXPECT_THROW(services->Dispatch(scsi_command::mode_sense10), scsi_exception)<< "Subpages are not supported";
 }
 
 TEST(HostServicesTest, SetUpModePages)

diff --git a/cpp/test/storage_device_test.cpp b/cpp/test/storage_device_test.cpp
@@ -451,6 +451,9 @@ TEST(StorageDeviceTest, ModeSense6)
     EXPECT_NO_THROW(device->Dispatch(scsi_command::mode_sense6));
     const auto &buf = controller->GetBuffer();
     EXPECT_EQ(0x80, buf[2]) << "Wrong device-specific parameter";
+
+    controller->SetCdbByte(3, 0x01);
+    EXPECT_THROW(device->Dispatch(scsi_command::mode_sense6), scsi_exception)<< "Subpages are not supported";
 }
 
 TEST(StorageDeviceTest, ModeSense10)
@@ -500,6 +503,9 @@ TEST(StorageDeviceTest, ModeSense10)
     EXPECT_NO_THROW(device->Dispatch(scsi_command::mode_sense10));
     buf = controller->GetBuffer();
     EXPECT_EQ(0x80, buf[3]) << "Wrong device-specific parameter";
+
+    controller->SetCdbByte(3, 0x01);
+    EXPECT_THROW(device->Dispatch(scsi_command::mode_sense10), scsi_exception)<< "Subpages are not supported";
 }
 
 TEST(StorageDeviceTest, GetStatistics)