Macintosh-HW
/
RASCSI
mirror of https://github.com/akuker/RASCSI.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Bugfix: MODE SELECT for format page is incorrect (issue #818) (#899) 

* Fix issue with MODE SELECT (#818)

* Replaced strncpy because it was causing a compilation issue
This commit is contained in:
Uwe Seimet 2022-10-10 08:16:47 +02:00 committed by GitHub
parent ccdb51b727
commit 4e4c5b205a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 119 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/raspberrypi/controllers/scsi_controller.cpp
View File

@ -625,6 +625,7 @@ void ScsiController::Receive()
}
// If result FALSE, move to status phase
// TODO Check whether we can raise scsi_exception here in order to simplify the error handling
if (!result) {
Error(sense_key::ABORTED_COMMAND);
return;
@ -648,7 +649,8 @@ void ScsiController::Receive()
break;
case BUS::phase_t::dataout:
FlushUnit();
// Block-oriented data have been handled above
DataOutNonBlockOriented();
Status();
break;
@ -769,47 +771,30 @@ bool ScsiController::XferOut(bool cont)
return false;
}
void ScsiController::FlushUnit()
void ScsiController::DataOutNonBlockOriented()
{
assert(IsDataOut());
auto disk = dynamic_pointer_cast<Disk>(GetDeviceForLun(GetEffectiveLun()));
if (disk == nullptr) {
return;
}
// WRITE system only
switch (GetOpcode()) {
case scsi_command::eCmdWrite6:
case scsi_command::eCmdWrite10:
case scsi_command::eCmdWrite16:
case scsi_command::eCmdWriteLong10:
case scsi_command::eCmdWriteLong16:
case scsi_command::eCmdVerify10:
case scsi_command::eCmdVerify16:
break;
case scsi_command::eCmdModeSelect6:
case scsi_command::eCmdModeSelect10:
// TODO What is this special handling of ModeSelect good for?
// Without it we would not need this method at all.
// ModeSelect is already handled in XferOutBlockOriented(). Why would it have to be handled once more?
try {
disk->ModeSelect(ctrl.cmd, GetBuffer(), GetOffset());
case scsi_command::eCmdModeSelect10: {
// TODO Try to get rid of this cast
if (auto device = dynamic_pointer_cast<ModePageDevice>(GetDeviceForLun(GetEffectiveLun()));
device != nullptr) {
device->ModeSelect(ctrl.cmd, GetBuffer(), GetOffset());
}
else {
throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_COMMAND_OPERATION_CODE);
}
}
catch(const scsi_exception& e) {
LOGWARN("Error occured while processing Mode Select command %02X\n", (int)GetOpcode())
Error(e.get_sense_key(), e.get_asc(), e.get_status());
return;
}
break;
break;
case scsi_command::eCmdSetMcastAddr:
// TODO: Eventually, we should store off the multicast address configuration data here...
break;
default:
LOGWARN("Received an unexpected flush command $%02X\n", (int)GetOpcode())
LOGWARN("Unexpected Data Out phase for command $%02X", (int)GetOpcode())
break;
}
}

2
src/raspberrypi/controllers/scsi_controller.h
View File

@ -103,7 +103,7 @@ private:
void ReceiveBytes();
void Execute();
void FlushUnit();
void DataOutNonBlockOriented();
void Receive();
void ProcessCommand();

2
src/raspberrypi/devices/device_factory.cpp
View File

@ -220,7 +220,7 @@ list<string> DeviceFactory::GetNetworkInterfaces() const
const int fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
ifreq ifr = {};
strncpy(ifr.ifr_name, tmp->ifa_name, IFNAMSIZ); //NOSONAR Using strncpy is safe here
strcpy(ifr.ifr_name, tmp->ifa_name); //NOSONAR Using strcpy is safe here
// Only list interfaces that are up
if (!ioctl(fd, SIOCGIFFLAGS, &ifr) && (ifr.ifr_flags & IFF_UP)) {
network_interfaces.emplace_back(tmp->ifa_name);

62
src/raspberrypi/devices/scsi_command_util.cpp
View File

@ -23,32 +23,30 @@ void scsi_command_util::ModeSelect(const vector<int>& cdb, const vector<BYTE>& b
throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_PARAMETER_LIST);
}
// Skip block descriptors
int offset;
if ((scsi_command)cdb[0] == scsi_command::eCmdModeSelect10) {
offset = 8 + GetInt16(buf, 6);
}
else {
offset = 4 + buf[3];
}
length -= offset;
bool has_valid_page_code = false;
// Mode Parameter header
int offset = 0;
if (length >= 12) {
// Check the block length
if (buf[9] != (BYTE)(sector_size >> 16) || buf[10] != (BYTE)(sector_size >> 8) ||
buf[11] != (BYTE)sector_size) {
// See below for details
LOGWARN("In order to change the sector size use the -b option when launching rascsi")
throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_PARAMETER_LIST);
}
offset += 12;
length -= 12;
}
// Parsing the page
// TODO The length handling is wrong in case of length < size
// Parse the pages
while (length > 0) {
// Format device page
if (int page = buf[offset]; page == 0x03) {
if (length < 14) {
throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::INVALID_FIELD_IN_PARAMETER_LIST);
}
// With this page the sector size for a subsequent FORMAT can be selected, but only very few
// drives support this, e.g FUJITSU M2624S
// We are fine as long as the current sector size remains unchanged
if (buf[offset + 0xc] != (BYTE)(sector_size >> 8) || buf[offset + 0xd] != (BYTE)sector_size) {
if (GetInt16(buf, offset + 12) != sector_size) {
// With rascsi it is not possible to permanently (by formatting) change the sector size,
// because the size is an externally configurable setting only
LOGWARN("In order to change the sector size use the -b option when launching rascsi")
@ -63,6 +61,7 @@ void scsi_command_util::ModeSelect(const vector<int>& cdb, const vector<BYTE>& b
// Advance to the next page
int size = buf[offset + 1] + 2;
length -= size;
offset += size;
}
@ -92,30 +91,45 @@ void scsi_command_util::AddAppleVendorModePage(map<int, vector<byte>>& pages, bo
// No changeable area
if (!changeable) {
const char APPLE_DATA[] = "APPLE COMPUTER, INC ";
memcpy(&buf[2], APPLE_DATA, sizeof(APPLE_DATA));
memcpy(&buf.data()[2], APPLE_DATA, sizeof(APPLE_DATA));
}
pages[48] = buf;
}
int scsi_command_util::GetInt16(const vector<BYTE>& buf, int offset)
{
assert(buf.size() > (size_t)offset + 1);
return ((int)buf[offset] << 8) | buf[offset + 1];
}
int scsi_command_util::GetInt16(const vector<int>& buf, int offset)
{
assert(buf.size() > (size_t)offset + 1);
return (buf[offset] << 8) | buf[offset + 1];
}
int scsi_command_util::GetInt24(const vector<int>& buf, int offset)
{
assert(buf.size() > (size_t)offset + 2);
return (buf[offset] << 16) | (buf[offset + 1] << 8) | buf[offset + 2];
}
uint32_t scsi_command_util::GetInt32(const vector<int>& buf, int offset)
{
assert(buf.size() > (size_t)offset + 3);
return ((uint32_t)buf[offset] << 24) | ((uint32_t)buf[offset + 1] << 16) |
((uint32_t)buf[offset + 2] << 8) | (uint32_t)buf[offset + 3];
}
uint64_t scsi_command_util::GetInt64(const vector<int>& buf, int offset)
{
assert(buf.size() > (size_t)offset + 7);
return ((uint64_t)buf[offset] << 56) | ((uint64_t)buf[offset + 1] << 48) |
((uint64_t)buf[offset + 2] << 40) | ((uint64_t)buf[offset + 3] << 32) |
((uint64_t)buf[offset + 4] << 24) | ((uint64_t)buf[offset + 5] << 16) |
@ -124,12 +138,16 @@ uint64_t scsi_command_util::GetInt64(const vector<int>& buf, int offset)
void scsi_command_util::SetInt16(vector<byte>& buf, int offset, int value)
{
assert(buf.size() > (size_t)offset + 1);
buf[offset] = (byte)(value >> 8);
buf[offset + 1] = (byte)value;
}
void scsi_command_util::SetInt32(vector<byte>& buf, int offset, uint32_t value)
{
assert(buf.size() > (size_t)offset + 3);
buf[offset] = (byte)(value >> 24);
buf[offset + 1] = (byte)(value >> 16);
buf[offset + 2] = (byte)(value >> 8);
@ -138,12 +156,16 @@ void scsi_command_util::SetInt32(vector<byte>& buf, int offset, uint32_t value)
void scsi_command_util::SetInt16(vector<BYTE>& buf, int offset, int value)
{
assert(buf.size() > (size_t)offset + 1);
buf[offset] = (BYTE)(value >> 8);
buf[offset + 1] = (BYTE)value;
}
void scsi_command_util::SetInt32(vector<BYTE>& buf, int offset, uint32_t value)
{
assert(buf.size() > (size_t)offset + 3);
buf[offset] = (BYTE)(value >> 24);
buf[offset + 1] = (BYTE)(value >> 16);
buf[offset + 2] = (BYTE)(value >> 8);
@ -152,6 +174,8 @@ void scsi_command_util::SetInt32(vector<BYTE>& buf, int offset, uint32_t value)
void scsi_command_util::SetInt64(vector<BYTE>& buf, int offset, uint64_t value)
{
assert(buf.size() > (size_t)offset + 7);
buf[offset] = (BYTE)(value >> 56);
buf[offset + 1] = (BYTE)(value >> 48);
buf[offset + 2] = (BYTE)(value >> 40);

1
src/raspberrypi/devices/scsi_command_util.h
View File

@ -22,6 +22,7 @@ namespace scsi_command_util
void EnrichFormatPage(map<int, vector<byte>>&, bool, int);
void AddAppleVendorModePage(map<int, vector<byte>>&, bool);
int GetInt16(const vector<BYTE>&, int);
int GetInt16(const vector<int>&, int);
int GetInt24(const vector<int>&, int);
uint32_t GetInt32(const vector<int>&, int);

68
src/raspberrypi/test/scsi_command_util_test.cpp
View File

@ -13,18 +13,20 @@
using namespace scsi_command_util;
TEST(ScsiCommandUtilTest, ModeSelect)
TEST(ScsiCommandUtilTest, ModeSelect6)
{
const int LENGTH = 12;
const int LENGTH = 26;
vector<int> cdb(16);
vector<BYTE> buf(255);
vector<int> cdb(6);
vector<BYTE> buf(LENGTH);
// PF (vendor-specific parameter format)
cdb[1] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 0), scsi_exception)
<< "Vendor-specific parameters are not supported";
cdb[0] = (int)scsi_command::eCmdModeSelect6;
cdb[0] = 0x15;
// PF (standard parameter format)
cdb[1] = 0x10;
// Request 512 bytes per sector
@ -35,18 +37,61 @@ TEST(ScsiCommandUtilTest, ModeSelect)
<< "Requested sector size does not match current sector size";
// Page 0
buf[LENGTH] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH + 2, 512), scsi_exception)
buf[12] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 512), scsi_exception)
<< "Unsupported page 0 was not rejected";
// Page 3 (Format Device Page)
buf[LENGTH] = 0x03;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH + 2, 512), scsi_exception)
buf[12] = 0x03;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 512), scsi_exception)
<< "Requested sector size does not match current sector size";
// Match the requested to the current sector size
buf[LENGTH + 12] = 0x02;
ModeSelect(cdb, buf, LENGTH + 2, 512);
buf[24] = 0x02;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH - 1, 512), scsi_exception)
<< "Not enough command parameters";
ModeSelect(cdb, buf, LENGTH, 512);
}
TEST(ScsiCommandUtilTest, ModeSelect10)
{
const int LENGTH = 30;
vector<int> cdb(10);
vector<BYTE> buf(LENGTH);
// PF (vendor-specific parameter format)
cdb[1] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 0), scsi_exception)
<< "Vendor-specific parameters are not supported";
cdb[0] = (int)scsi_command::eCmdModeSelect10;
// PF (standard parameter format)
cdb[1] = 0x10;
// Request 512 bytes per sector
buf[13] = 0x00;
buf[14] = 0x02;
buf[15] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 256), scsi_exception)
<< "Requested sector size does not match current sector size";
// Page 0
buf[16] = 0x00;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 512), scsi_exception)
<< "Unsupported page 0 was not rejected";
// Page 3 (Format Device Page)
buf[16] = 0x03;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH, 512), scsi_exception)
<< "Requested sector size does not match current sector size";
// Match the requested to the current sector size
buf[28] = 0x02;
EXPECT_THROW(ModeSelect(cdb, buf, LENGTH - 1, 512), scsi_exception)
<< "Not enough command parameters";
ModeSelect(cdb, buf, LENGTH, 512);
}
TEST(ScsiCommandUtilTest, EnrichFormatPage)
@ -85,6 +130,9 @@ TEST(ScsiCommandUtilTest, AddAppleVendorModePage)
TEST(ScsiCommandUtilTest, GetInt16)
{
vector<BYTE> b = { 0xfe, 0xdc };
EXPECT_EQ(0xfedc, GetInt16(b, 0));
vector<int> v = { 0x12, 0x34 };
EXPECT_EQ(0x1234, GetInt16(v, 0));
}