Skip to content

disk: fix data corruption on 256-byte sector images#362

Open
erichelgeson wants to merge 1 commit intomainfrom
eric/fix-256-byte
Open

disk: fix data corruption on 256-byte sector images#362
erichelgeson wants to merge 1 commit intomainfrom
eric/fix-256-byte

Conversation

@erichelgeson
Copy link
Copy Markdown
Contributor

@erichelgeson erichelgeson commented Apr 24, 2026

Two related bugs surfaced on images formatted with 256-byte SCSI sectors.

  1. start_dataInTransfer() used readSectorsDirect with count >> 9 when fastseek was enabled, silently dropping the trailing 256 bytes on odd-block transfers and reading the wrong 512 bytes when the file position was not 512-aligned. Gate the direct-read path on bytesPerSector == SD_SECTOR_SIZE; 256-byte images take the regular img.file.read() path.

  2. ImageBackingStore::read/write kept m_fsfile's position stale while serving contiguous raw-block I/O. On the first unaligned access the code flipped m_iscontiguous=false and fell back to m_fsfile still sitting at offset 0, corrupting both reads (wrong data) and writes (overwrote the image header). Sync m_fsfile from m_cursector before the mode transition in both read() and write().

Replaces #348

Two related bugs surfaced on images formatted with 256-byte SCSI
sectors (fixes from PR #348, simplified):

1. start_dataInTransfer() used readSectorsDirect with count >> 9 when
   fastseek was enabled, silently dropping the trailing 256 bytes on
   odd-block transfers and reading the wrong 512 bytes when the file
   position was not 512-aligned. Gate the direct-read path on
   bytesPerSector == SD_SECTOR_SIZE; 256-byte images take the regular
   img.file.read() path.

2. ImageBackingStore::read/write kept m_fsfile's position stale while
   serving contiguous raw-block I/O. On the first unaligned access
   the code flipped m_iscontiguous=false and fell back to m_fsfile
   still sitting at offset 0, corrupting both reads (wrong data) and
   writes (overwrote the image header). Sync m_fsfile from m_cursector
   before the mode transition in both read() and write().

Co-Authored-By: Per Mårtensson <pm@sweproj.com>
@github-actions
Copy link
Copy Markdown

BlueSCSI Memory Report

Compared against release v2026.03.01

Memory Usage

Target FLASH RAM SCRATCH_X SCRATCH_Y
Pico_2_Audio_SPDIF 674.8 KB (+54.8 KB) [33%] 511.7 KB [100%] 3.5 KB [88%] 3.8 KB (+1.0 KB) [94%]
Pico_2_DaynaPORT 667.1 KB (+53.7 KB) [33%] 511.7 KB [100%] 3.5 KB [88%] 3.0 KB (+1.0 KB) [75%]
Pico_Audio_SPDIF 696.3 KB (+62.5 KB) [34%] 252.3 KB (+6.8 KB) [99%] 1.5 KB [38%] 768 B [19%]
Pico_DaynaPORT 688.1 KB (+61.9 KB) [34%] 246.0 KB (+15.8 KB) [96%] 1.5 KB [38%]
Ultra 684.1 KB (+59.1 KB) [33%] 511.7 KB [100%] 3.5 KB [88%] 3.0 KB (+1.0 KB) [75%]
Ultra_Wide 444.0 KB (+63.1 KB) [22%] 511.7 KB [100%] 2.0 KB [50%] 3.0 KB (+1.0 KB) [75%]

Symbol Region Changes

Pico_2_Audio_SPDIF: 3 symbols moved FLASH → RAM (+12 B)
Symbol Size
operator delete(void*, unsigned int) 4 B
operator new[](unsigned int) 4 B
operator delete[](void*) 4 B
Pico_2_Audio_SPDIF: 9 symbols moved RAM → FLASH (-298 B RAM)
Symbol Size
tud_descriptor_device_cb 116 B
tud_descriptor_string_cb 96 B
_fstat 16 B
_lseek 16 B
_close 16 B
_isatty 16 B
tud_descriptor_configuration_cb 12 B
_read 6 B
_write 4 B
Pico_2_DaynaPORT: 3 symbols moved FLASH → RAM (+12 B)
Symbol Size
operator delete(void*, unsigned int) 4 B
operator new[](unsigned int) 4 B
operator delete[](void*) 4 B
Pico_2_DaynaPORT: 9 symbols moved RAM → FLASH (-298 B RAM)
Symbol Size
tud_descriptor_device_cb 116 B
tud_descriptor_string_cb 96 B
_fstat 16 B
_lseek 16 B
_close 16 B
_isatty 16 B
tud_descriptor_configuration_cb 12 B
_read 6 B
_write 4 B
Pico_Audio_SPDIF: 13 symbols moved FLASH → RAM (+2.0 KB)
Symbol Size
mscd_xfer_cb 1.4 KB
mscd_control_xfer_cb 328 B
tud_descriptor_string_cb 116 B
mscd_open 104 B
mscd_reset 20 B
mscd_init 20 B
tud_descriptor_configuration_cb 8 B
operator delete(void*, unsigned int) 8 B
operator new[](unsigned int) 8 B
tud_descriptor_device_cb 8 B
operator delete[](void*) 8 B
exit 6 B
SdSpiCard::errorCode() const 4 B
Pico_Audio_SPDIF: 43 symbols moved RAM → FLASH (-5.3 KB RAM)
Symbol Size
amigaWifiCommand 1.2 KB
FatFile::rename(FatFile*, char const*) 332 B
FatPartition::init(FsBlockDeviceInterface*, unsigned char, unsigned long) 324 B
ExFatPartition::init(FsBlockDeviceInterface*, unsigned char, unsigned long) 252 B
ExFatFile::addDirCluster() 196 B
ExFatPartition::bitmapFind(unsigned long, unsigned long) 192 B
partitionTableGetVolumeStartSector(FsCache&, unsigned char) 192 B
ExFatPartition::bitmapModify(unsigned long, unsigned long, bool) 164 B
FatFile::makeUniqueSfn(FatLfn_t*) 144 B
ExFatFile::rename(ExFatFile*, char const*) 140 B
ExFatFile::preAllocate(unsigned long long) 136 B
ExFatPartition::freeClusterCount() 136 B
FatPartition::allocContiguous(unsigned long, unsigned long*) 136 B
FatPartition::freeClusterCount() 124 B
FatPartition::allocateCluster(unsigned long, unsigned long*) 124 B
FatFile::addDirCluster() 112 B
FatFile::fgets(char*, int, char const*) 106 B
FatFile::contiguousRange(unsigned long*, unsigned long*) 106 B
ExFatFile::fgets(char*, int, char const*) 104 B
FatPartition::fatPut(unsigned long, unsigned long) 104 B
FsBaseFile::copy(FsBaseFile const*) 102 B
ExFatPartition::fatPut(unsigned long, unsigned long) 80 B
ExFatPartition::freeChain(unsigned long) 78 B
ExFatFile::contiguousRange(unsigned long*, unsigned long*) 76 B
FatVolume::chdir(char const*) 72 B
FatFile::preAllocate(unsigned long) 72 B
ExFatVolume::chdir(char const*) 68 B
FatPartition::freeChain(unsigned long) 58 B
FatFile::getLfnChar(DirLfn_t const*, unsigned char) 52 B
ExFatFile::peek() 50 B
... and 13 more
Pico_DaynaPORT: 34 symbols moved FLASH → RAM (+7.2 KB)
Symbol Size
mscd_xfer_cb 1.4 KB
ExFatFile::write(void const*, unsigned int) 992 B
ExFatFile::read(void*, unsigned int) 672 B
FatFile::write(void const*, unsigned int) 524 B
FatFile::readPrivate(void*, unsigned int, DirFat_t**) 498 B
ExFatFile::seekSet(unsigned long long) 468 B
ExFatFile::buildSectorMap() 444 B
FatFile::buildSectorMap() 358 B
mscd_control_xfer_cb 328 B
FatFile::seekSet(unsigned long) 272 B
FatFile::readSectorsDirect(unsigned long, unsigned char*, unsigned long) 200 B
ExFatFile::readSectorsDirect(unsigned long, unsigned char*, unsigned long) 200 B
FatPartition::fatGet(unsigned long, unsigned long*) 154 B
tud_descriptor_string_cb 116 B
mscd_open 104 B
ExFatPartition::fatGet(unsigned long, unsigned long*) 96 B
FsCache::prepare(unsigned long, unsigned char) 84 B
ExFatPartition::dirSeek(DirPos_t*, unsigned long) 80 B
FsCache::sync() 74 B
ExFatPartition::dirCache(DirPos_t const*, unsigned char) 68 B
ExFatFile::dirCache(unsigned char, unsigned char) 60 B
FatFile::cacheDirEntry(unsigned char) 32 B
FatFile::readDirCache() 32 B
mscd_reset 20 B
mscd_init 20 B
FsBaseFile::operator=(FsBaseFile const&) 12 B
tud_descriptor_configuration_cb 8 B
operator delete(void*, unsigned int) 8 B
operator new[](unsigned int) 8 B
tud_descriptor_device_cb 8 B
... and 4 more
Pico_DaynaPORT: 3 symbols moved RAM → FLASH (-1.2 KB RAM)
Symbol Size
amigaWifiCommand 1.2 KB
StreamFile<FsBaseFile, unsigned long long>::peek() 34 B
StreamFile<FatFile, unsigned long>::peek() 10 B
Ultra: 4 symbols moved FLASH → RAM (+16 B)
Symbol Size
operator new(unsigned int) 4 B
operator delete[](void*) 4 B
operator new[](unsigned int) 4 B
operator delete(void*, unsigned int) 4 B
Ultra: 9 symbols moved RAM → FLASH (-298 B RAM)
Symbol Size
tud_descriptor_device_cb 116 B
tud_descriptor_string_cb 96 B
_lseek 16 B
_close 16 B
_fstat 16 B
_isatty 16 B
tud_descriptor_configuration_cb 12 B
_read 6 B
_write 4 B
Ultra_Wide: 4 symbols moved FLASH → RAM (+16 B)
Symbol Size
operator delete(void*, unsigned int) 4 B
operator new[](unsigned int) 4 B
operator delete[](void*) 4 B
operator new(unsigned int) 4 B
Ultra_Wide: 9 symbols moved RAM → FLASH (-298 B RAM)
Symbol Size
tud_descriptor_device_cb 116 B
tud_descriptor_string_cb 96 B
_fstat 16 B
_lseek 16 B
_close 16 B
_isatty 16 B
tud_descriptor_configuration_cb 12 B
_read 6 B
_write 4 B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant