diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 534962da0..1f1d9b6c6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -105,6 +105,12 @@ jobs: -v /opt/build-tools:/mnt/build-tools:ro \ -v $GITHUB_WORKSPACE:/mnt/project:rw my_docker_image /bin/bash -c "cd /mnt/project && make -B -C target/pc/linux/test_http_target test-integer-widths" + - name: Test G64 track bounds + run: | + docker run --rm \ + -v /opt/build-tools:/mnt/build-tools:ro \ + -v $GITHUB_WORKSPACE:/mnt/project:rw my_docker_image /bin/bash -c "cd /mnt/project && make -C software/drive/tests" + - name: Test UCI Palette Commands run: | docker run --rm \ diff --git a/run-tests b/run-tests index 4b87a1bf2..3a1d55e89 100755 --- a/run-tests +++ b/run-tests @@ -295,6 +295,13 @@ SUITES: Sequence[Suite] = ( # oversized D81 and proves the bounded layout leaves the device usable. Suite("e2e", "d81-track-bounds", "tests/e2e/drive/d81_track_bounds_test.py", "-H @HOST@ -p @PASS@ -t @TIMEOUT@ --test all", profile=profiles.QUICK), + # Mounts a G71 whose MFM-marked track is malformed -- one declaring a + # single byte where the metadata area alone is 162, and one declaring + # 0x4123, which is 16675 read as a length and 291 read as fourteen bits -- + # and proves the GCR side of the same disk still reads afterwards. The + # first exercise of the mixed GCR/MFM path on real hardware. + Suite("e2e", "g71-mfm-bounds", "tests/e2e/drive/g71_mfm_bounds_test.py", + "-H @HOST@ -p @PASS@ -t @TIMEOUT@", profile=profiles.QUICK), # Calls every endpoint the firmware registers, and fails when one is # neither probed nor excluded with a reason. The lockup that prompted it # shipped because no suite called that route at all, so the gate matters diff --git a/software/drive/c1541.cc b/software/drive/c1541.cc index 84cdb03ae..74f752015 100644 --- a/software/drive/c1541.cc +++ b/software/drive/c1541.cc @@ -3,6 +3,7 @@ #include #include #include +#include "gcr_track_bounds.h" #include "itu.h" #include "dump_hex.h" @@ -461,40 +462,30 @@ void C1541 :: insert_disk(bool protect, GcrImage *image) uint32_t rotation_speed = (CLOCK_FREQ / 20); // 2 (half clocks) * 1/8 (bytes) * clocks per track. 300 RPM = 5 RPS. (5 * 8 / 2) = 20 // side 0 - uint32_t bit_time; - uint32_t track_len; volatile uint32_t *param = (volatile uint32_t *)®isters[C1541_PARAM_RAM]; for(int i=0; i < GCRIMAGE_FIRSTTRACKSIDE1; i++) { GcrTrack *tr = &image->tracks[i]; - if (tr->track_address) { - bit_time = rotation_speed / tr->track_length; - track_len = tr->track_length; - // printf("Side0: %2d %08x %08x %d\n", i, tr->track_address, track_len, bit_time); - *(param++) = (uint32_t)tr->track_address; - *(param++) = (track_len-1) | (bit_time << 16); - } else { - // printf("Side0: %2d %08x %08x %d\n", i, tr->track_address, track_len, bit_time); - *(param++) = (uint32_t)dummy_track; - *(param++) = (track_len-1) | (bit_time << 16); - } + uint32_t track_addr, track_param; + gcr_track_parameters((uint32_t)tr->track_address, tr->track_length, + (uint32_t)dummy_track, GCRIMAGE_DUMMYTRACKLEN, + rotation_speed, &track_addr, &track_param); + // printf("Side0: %2d %08x %08x\n", i, track_addr, track_param); + *(param++) = track_addr; + *(param++) = track_param; registers[C1541_DIRTYFLAGS + i/2] = 0; - } + } // Side 1 param = (volatile uint32_t *)®isters[C1541_PARAM_RAM + 0x400]; // side 1 for(int i=GCRIMAGE_FIRSTTRACKSIDE1; i < GCRIMAGE_MAXHDRTRACKS; i++) { GcrTrack *tr = &image->tracks[i]; - if (tr->track_address) { - bit_time = rotation_speed / tr->track_length; - track_len = tr->track_length; - // printf("Side1: %2d %08x %08x %d\n", i, tr->track_address, tr->track_length, bit_time); - *(param++) = (uint32_t)tr->track_address; - *(param++) = (track_len-1) | (bit_time << 16); - } else { - // printf("Side1: %2d %08x %08x %d\n", i, tr->track_address, tr->track_length, bit_time); - *(param++) = (uint32_t)dummy_track; - *(param++) = (track_len-1) | (bit_time << 16); - } + uint32_t track_addr, track_param; + gcr_track_parameters((uint32_t)tr->track_address, tr->track_length, + (uint32_t)dummy_track, GCRIMAGE_DUMMYTRACKLEN, + rotation_speed, &track_addr, &track_param); + // printf("Side1: %2d %08x %08x\n", i, track_addr, track_param); + *(param++) = track_addr; + *(param++) = track_param; registers[C1541_DIRTYFLAGS + i/2] = 0; } @@ -527,7 +518,14 @@ void C1541 :: map_gcr_image_to_mfm(void) if (!gcr || !mtr) { continue; } - mtr->reservedSpace = gtr->track_length - MFM_TRACK_HEADER_SIZE; + // A track shorter than the metadata area cannot carry one, whatever + // its marker says, and there is no space behind an area that does + // not fit. Leave the entry as init() zeroed it: no sectors, no + // reserved space, so UpdateTrack() admits no write for this track. + if (!gcr_track_can_hold_mfm_header(gtr->track_length, MFM_TRACK_HEADER_SIZE)) { + continue; + } + mtr->reservedSpace = gcr_mfm_reserved_space(gtr->track_length, MFM_TRACK_HEADER_SIZE); mtr->offsetInFile = (int)gcr - (int)gcr_image->gcr_data; mtr->offsetInFile += MFM_TRACK_HEADER_SIZE; @@ -615,7 +613,16 @@ void C1541 :: mount_g64(bool protect, File *file) return; } printf("Loading..."); - gcr_image->load(file); + if (!gcr_image->load(file)) { + // load() has already invalidated the image, so its track table holds + // nothing that may be programmed. remove_disk() ran above, so an empty + // drive is the state the machine is already in. + printf("Failed. Not a G64 image, or its track table does not fit.\n"); + fm->fclose(mount_file); + mount_file = NULL; + drive_reset(0); // do not reset, but restore the freeze + return; + } printf("Inserting..."); insert_disk(protect, gcr_image); printf("Done\n"); diff --git a/software/drive/disk_image.cc b/software/drive/disk_image.cc index 05fad24c8..b9dc2ffbd 100644 --- a/software/drive/disk_image.cc +++ b/software/drive/disk_image.cc @@ -20,6 +20,7 @@ extern "C" { #include "user_file_interaction.h" #include "blockdev_file.h" #include "endianness.h" +#include "gcr_track_bounds.h" #define HARDWARE_ENCODING 1 @@ -709,23 +710,32 @@ bool GcrImage :: load(File *f) // track offsets start at 0x000c for(int i=0;i GCRIMAGE_MAXSIZE) { + if(offset >= GCRIMAGE_MAXSIZE) { printf("Error. Track pointer outside GCR memory range.\n"); return false; } + // Offset zero means the track is absent. A non-zero one still has to + // point at two bytes the file actually delivered, since reading the + // length word is itself an access into the image buffer. + if(!gcr_track_header_is_readable(offset, bytes_read)) { + continue; + } tr = gcr_data + offset; - if(offset) { - w = tr[0] | (uint16_t(tr[1]) << 8); - tracks[i].track_address = tr + 2; - tracks[i].track_length = (int)(w & 0x3FFF); - tracks[i].track_used = true; - tracks[i].in_image_file = true; - tracks[i].track_is_mfm = (w & 0x8000); -// printf("Set track %d.%d to 0x%6x / 0x%4x.\n", (i>>1)+1, (i&1)?5:0, tracks[i].track_address, w); - if (i >= GCRIMAGE_FIRSTTRACKSIDE1) { - double_sided = true; - } - } + w = tr[0] | (uint16_t(tr[1]) << 8); + int length = gcr_validated_track_length(w, offset, bytes_read, GCRIMAGE_MAXTRACKLEN); + if(!length) { + printf("Track %d: declared length %d was not delivered by the file. Track skipped.\n", + i, (int)(w & 0x7FFF)); + continue; // invalidate() left this entry unused; leave it that way + } + tracks[i].track_address = tr + 2; + tracks[i].track_length = length; + tracks[i].track_used = true; + tracks[i].in_image_file = true; + tracks[i].track_is_mfm = (w & 0x8000); + if (i >= GCRIMAGE_FIRSTTRACKSIDE1) { + double_sided = true; + } } // Track speed zone info starts after the track offsets uint8_t reported_tracks = gcr_data[9]; diff --git a/software/drive/disk_image.h b/software/drive/disk_image.h index 8111b9bd4..19adf4f84 100644 --- a/software/drive/disk_image.h +++ b/software/drive/disk_image.h @@ -10,6 +10,7 @@ #include "menu.h" #include "filemanager.h" #include "subsys.h" +#include "mfmdisk.h" // WD_MAX_SECTORS_PER_TRACK, used by MFM_TRACK_HEADER_SIZE below #define GCR_DECODER_GCR_IN (*(volatile uint8_t *)(GCR_CODER_BASE + 0x00)) #define GCR_DECODER_BIN_OUT0 (*(volatile uint8_t *)(GCR_CODER_BASE + 0x00)) @@ -48,7 +49,14 @@ #define C1541_MIN_D71_SIZE (2*C1541_MAX_D64_35_NO_ERRORS) #define C1541_D71_SIZE_WITH_ERRORS (C1541_MIN_D71_SIZE + 1366) -#define MFM_TRACK_HEADER_SIZE 2 + (WD_MAX_SECTORS_PER_TRACK * 5) +/* Parenthesised because it is subtracted: without the outer brackets + * "x - MFM_TRACK_HEADER_SIZE" expanded to "x - 2 + 160", which is x + 158 + * rather than x - 162, and map_gcr_image_to_mfm() reserved 320 bytes more + * than the track had. The static_assert below fails if they are lost again; + * comparing the macro against 162 would not, since 2 + 160 equals 162. */ +#define MFM_TRACK_HEADER_SIZE (2 + (WD_MAX_SECTORS_PER_TRACK * 5)) +static_assert(0 - MFM_TRACK_HEADER_SIZE == -162, + "MFM_TRACK_HEADER_SIZE must survive being subtracted"); class BinImage; diff --git a/software/drive/gcr_track_bounds.h b/software/drive/gcr_track_bounds.h new file mode 100644 index 000000000..422213b47 --- /dev/null +++ b/software/drive/gcr_track_bounds.h @@ -0,0 +1,145 @@ +/* + * gcr_track_bounds.h -- what a G64 says about its tracks, checked. + * + * The track lengths in a G64 file are not decoration: insert_disk() programs + * them into the drive engine's parameter RAM, and floppy_mem.vhd both reads + * and writes memory bounded by them. So a length taken from the file decides + * how far the hardware reaches into a buffer, and a track that is not in the + * image at all still has to be given a parameter pair the engine can survive. + * + * These are free functions over plain integers, deliberately: none of the + * drive headers can be compiled on a build host, and this is the part that is + * worth testing there. See software/drive/tests/. + */ +#ifndef DRIVE_GCR_TRACK_BOUNDS_H +#define DRIVE_GCR_TRACK_BOUNDS_H + +#include + +/* The low half of a parameter word is the last valid offset in the track, so + * this is a 257-byte empty track. init() and remove_disk() already program it + * for a drive with nothing in it. */ +#define GCR_EMPTY_TRACK_PARAM 0x100 + +/* A track table entry points at a two-byte length word followed by the track + * data. Reading that word is itself an access into the image buffer, so it has + * to lie inside the part of the buffer the file actually filled. Offset zero + * means the track is absent. */ +static inline bool gcr_track_header_is_readable(uint32_t offset, uint32_t bytes_read) +{ + if (offset == 0 || offset > bytes_read) { + return false; + } + return (bytes_read - offset) >= 2; +} + +/* Returns the track length to use, or 0 when the track cannot be used and has + * to be left marked absent. + * + * declared the 16-bit word from the image, flag bits included + * offset byte offset of that word within the buffer + * bytes_read how much of the buffer the file actually filled + * max_length longest track the format allows + * + * The bound is what was read, not how large the buffer is. A truncated image + * can declare a track whose header is present but whose data never arrived, + * and the buffer behind it holds whatever the previous mount left there. So + * the whole track, header word included, has to lie inside what was read: + * offset + 2 + length <= bytes_read, written as subtractions so that no sum + * can wrap. + * + * The length is taken from fifteen bits, not fourteen. Bit 15 is the MFM + * marker this firmware writes and Denise reads; nothing gives bit 14 a + * meaning, so it belongs to the length. write_track() and save() already + * write the length unmasked and set bit 15 on top of it, so reading fourteen + * bits meant the image could not be read back as it was written: a declared + * 0x4123 came out as 0x0123, small enough to pass every check below and be + * programmed as a 291 byte track. + * + * The width is not arbitrary -- the drive's own parameter field is fourteen + * bits -- but the file and the engine are two different things, and masking + * at parse time conflates them. The drive engine's own ceiling is 16383: + * max_offset in floppy_param_mem.vhd is fourteen bits wide, and + * floppy_mem.vhd wraps offset_count at it for reads and writes alike. That + * ceiling never comes into play here, because max_length -- 7928 for a G64, + * GCRIMAGE_MAXTRACKLEN -- is less than half of it and is checked below. So + * the parse stays faithful to the file and the bound does the rejecting. + */ +static inline int gcr_validated_track_length(uint16_t declared, uint32_t offset, + uint32_t bytes_read, int max_length) +{ + int length = (int)(declared & 0x7FFF); + + if (length <= 0 || length > max_length) { + return 0; /* zero would also divide by zero in insert_disk() */ + } + if (offset > bytes_read || (bytes_read - offset) < 2) { + return 0; + } + if ((uint32_t)length > (bytes_read - offset - 2)) { + return 0; + } + return length; +} + +/* An MFM track carries a metadata area at its front: a sector count, a version + * byte, and five bytes for each of up to WD_MAX_SECTORS_PER_TRACK sectors. + * map_gcr_image_to_mfm() reads that area whole for a track marked MFM, and + * reserves what lies behind it for sector data. Neither exists unless the + * track is at least as long as the area itself. + * + * Length and MFM marker share one word in a G64, so 0x8001 -- a one-byte track + * marked MFM -- is a legal thing to put in an image. The declared extent then + * checks out: one byte was delivered. The mapping used to believe the marker + * anyway, read the full metadata area from that one byte, and take the space + * behind it as a subtraction in an unsigned field, where it cannot come out + * negative. + * + * track_length the validated length of the track + * header_size bytes of metadata at the front of an MFM track + */ +static inline bool gcr_track_can_hold_mfm_header(int track_length, int header_size) +{ + if (header_size <= 0 || track_length <= 0) { + return false; + } + return track_length >= header_size; +} + +/* Bytes behind the metadata area, or 0 for a track with no room for one. The + * caller keeps this in an unsigned field that gates every track write, so it + * must never be reached by underflow. */ +static inline uint32_t gcr_mfm_reserved_space(int track_length, int header_size) +{ + if (!gcr_track_can_hold_mfm_header(track_length, header_size)) { + return 0; + } + return (uint32_t)(track_length - header_size); +} + +/* Builds one entry of the drive's track parameter RAM: the address the engine + * works at, and a word carrying the last valid offset in the low half and the + * bit time in the high half. + * + * A track that is not in the image gets the dummy track together with the same + * short, safe length init() and remove_disk() use -- not whatever length the + * previous track happened to leave in a local. + */ +static inline void gcr_track_parameters(uint32_t track_address, int track_length, + uint32_t dummy_address, int dummy_length, + uint32_t rotation_speed, + uint32_t *out_address, uint32_t *out_param) +{ + if (track_address != 0 && track_length > 0) { + uint32_t bit_time = rotation_speed / (uint32_t)track_length; + *out_address = track_address; + *out_param = (uint32_t)(track_length - 1) | (bit_time << 16); + return; + } + + uint32_t bit_time = (dummy_length > 0) ? (rotation_speed / (uint32_t)dummy_length) : 0; + *out_address = dummy_address; + *out_param = GCR_EMPTY_TRACK_PARAM | (bit_time << 16); +} + +#endif /* DRIVE_GCR_TRACK_BOUNDS_H */ diff --git a/software/drive/tests/Makefile b/software/drive/tests/Makefile new file mode 100644 index 000000000..e9669a892 --- /dev/null +++ b/software/drive/tests/Makefile @@ -0,0 +1,15 @@ +CXX ?= g++ +CXXFLAGS := -std=c++14 -g -Wall -Wextra -I. -I.. -I../../io/usb/tests +OUTPUT_DIR := output +GCR_TRACK_BOUNDS_TEST := $(OUTPUT_DIR)/gcrTrackBoundsTest + +.PHONY: all gcr-track-bounds clean + +all: gcr-track-bounds + +gcr-track-bounds: + mkdir -p $(OUTPUT_DIR) + $(CXX) $(CXXFLAGS) gcr_track_bounds_test.cpp ../../io/usb/tests/host_test_main.cpp -lpthread -o $(GCR_TRACK_BOUNDS_TEST) && $(GCR_TRACK_BOUNDS_TEST) + +clean: + rm -rf $(OUTPUT_DIR) diff --git a/software/drive/tests/gcr_track_bounds_test.cpp b/software/drive/tests/gcr_track_bounds_test.cpp new file mode 100644 index 000000000..c33ec21e4 --- /dev/null +++ b/software/drive/tests/gcr_track_bounds_test.cpp @@ -0,0 +1,319 @@ +// Regression tests for GideonZ/1541ultimate#823: a G64's track metadata used to +// program the drive engine's memory bounds without ever being checked against +// the buffer that holds the image. +// +// The four problems in that issue, and where each is pinned down below: +// +// 1. parameter RAM programmed from uninitialised locals -> TrackParameters +// 2. track length never validated against the buffer -> ValidatedLength +// 3. half-tracks inherit the previous track's length -> TrackParameters +// 4. divide by zero on a zero-length track -> ValidatedLength +// +// And what checking the buffer alone still let through, found in review of the +// first fix: a truncated file whose track header arrived but whose track data +// did not -> TruncatedImage. +// +// And what checking the declared extent still let through, found in review of +// the second fix: a track short enough that the MFM mapping's own, larger +// assumption about it does not hold -> MfmHeader. + +#include "../../io/usb/tests/host_test/host_test.h" +#include "../gcr_track_bounds.h" + +namespace { + +// The real constants, so the numbers here are the ones the firmware uses. +const uint32_t kMaxSize = (0x1EF8 * 80) + (12 + (168 * 10)); // GCRIMAGE_MAXSIZE +const int kMaxTrackLen = 0x1EF8; // GCRIMAGE_MAXTRACKLEN +const int kDummyLen = 0x1E0C; // GCRIMAGE_DUMMYTRACKLEN +const uint32_t kRotationSpeed = 50000000 / 20; // CLOCK_FREQ / 20 on the U64 +const int kMfmHeaderSize = 2 + (32 * 5); // MFM_TRACK_HEADER_SIZE + +} // namespace + +// ---------------------------------------------------------------- problem 2 -- +// +// The third argument is how much of the buffer the file filled. Passing +// kMaxSize here models an image that filled it completely, which is what the +// buffer-overrun cases below are about; the truncated cases further down pass +// the smaller number a short file leaves behind. + +TEST(ValidatedLength, AcceptsALegitimateTrack) +{ + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 0x2000, kMaxSize, kMaxTrackLen), 0x1E0C); +} + +TEST(ValidatedLength, KeepsTheLongestLegalTrack) +{ + EXPECT_EQ(gcr_validated_track_length(kMaxTrackLen, 0x2000, kMaxSize, kMaxTrackLen), kMaxTrackLen); +} + +TEST(ValidatedLength, IgnoresTheFlagBitsAboveTheLength) +{ + // Bit 15 is the MFM marker; it must not be read as part of the length. + EXPECT_EQ(gcr_validated_track_length(0x8000 | 0x1E0C, 0x2000, kMaxSize, kMaxTrackLen), 0x1E0C); +} + +TEST(ValidatedLength, ReadsBitFourteenAsLengthAndRejectsWhatItMakes) +{ + // Nothing gives bit 14 a meaning, and the writer sets neither a mask nor a + // flag there, so it is part of the length. Masking it away turned 0x4123 + // into 291 -- a plausible track that passes every check and gets + // programmed. Read as length it is 16675, above the format maximum, and + // the track is refused instead. + EXPECT_EQ(gcr_validated_track_length(0x4123, 0x2000, kMaxSize, kMaxTrackLen), 0); +} + +TEST(ValidatedLength, ReadsBitFourteenAsLengthWithTheMfmMarkerSet) +{ + // Both flag positions at once: the marker is stripped, the rest is length. + EXPECT_EQ(gcr_validated_track_length(0x8000 | 0x4123, 0x2000, kMaxSize, kMaxTrackLen), 0); +} + +TEST(ValidatedLength, RejectsALengthAboveTheFormatMaximum) +{ + // 14 bits admit 16383, more than twice the longest real track. Before the + // fix this was programmed as-is and the engine read and wrote past the + // allocation. + EXPECT_EQ(gcr_validated_track_length(0x3FFF, 0x2000, kMaxSize, kMaxTrackLen), 0); +} + +TEST(ValidatedLength, RejectsATrackRunningOffTheEndOfTheBuffer) +{ + // Legal length, but placed so that it does not fit behind its own header. + const uint32_t offset = kMaxSize - 100; + EXPECT_EQ(gcr_validated_track_length(0x1E0C, offset, kMaxSize, kMaxTrackLen), 0); +} + +TEST(ValidatedLength, AcceptsATrackEndingExactlyAtTheBufferEnd) +{ + const uint32_t offset = kMaxSize - 0x1E0C - 2; + EXPECT_EQ(gcr_validated_track_length(0x1E0C, offset, kMaxSize, kMaxTrackLen), 0x1E0C); +} + +TEST(ValidatedLength, RejectsATrackOneByteTooLongForTheBuffer) +{ + const uint32_t offset = kMaxSize - 0x1E0C - 1; + EXPECT_EQ(gcr_validated_track_length(0x1E0C, offset, kMaxSize, kMaxTrackLen), 0); +} + +TEST(ValidatedLength, SurvivesAnOffsetPastTheBuffer) +{ + // The caller rejects these first, but the arithmetic here must not wrap. + EXPECT_EQ(gcr_validated_track_length(0x1E0C, kMaxSize + 1, kMaxSize, kMaxTrackLen), 0); + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 0xFFFFFFFFu, kMaxSize, kMaxTrackLen), 0); + EXPECT_EQ(gcr_validated_track_length(0x1E0C, kMaxSize - 1, kMaxSize, kMaxTrackLen), 0); +} + +// ------------------------------------------------------------ truncation -- +// +// Reported by @chrisgleissner against the first version of this fix, which +// bounded the track by the size of the buffer rather than by what the file +// delivered. The buffer is not zeroed between mounts, so what lies past the +// end of a short file is the previous image. + +TEST(TruncatedImage, RejectsATrackWhoseDataNeverArrived) +{ + // The reported case, exactly: a 14-byte GCR-1541 file. Track 0 points at + // offset 12, the two length bytes are there, and they declare a full + // 0x1E0C track. Against the buffer this passes; against the file it must + // not, or gcr_data+14 .. gcr_data+7705 go to the drive unread. + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, 14, kMaxTrackLen), 0); +} + +TEST(TruncatedImage, RejectsATrackCutShortByOneByte) +{ + const uint32_t bytes_read = 12 + 2 + 0x1E0C - 1; + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, bytes_read, kMaxTrackLen), 0); +} + +TEST(TruncatedImage, AcceptsATrackEndingExactlyWhereTheFileDoes) +{ + const uint32_t bytes_read = 12 + 2 + 0x1E0C; + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, bytes_read, kMaxTrackLen), 0x1E0C); +} + +TEST(TruncatedImage, DoesNotCareHowLargeTheBufferIs) +{ + // Same track, same short file. The only difference between these two calls + // used to be the answer: the buffer was the bound, so the whole image was + // judged by memory that had nothing to do with it. + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, kMaxSize, kMaxTrackLen), 0x1E0C); + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, 14, kMaxTrackLen), 0); +} + +TEST(TruncatedImage, SurvivesAnEmptyRead) +{ + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 12, 0, kMaxTrackLen), 0); + EXPECT_EQ(gcr_validated_track_length(0x1E0C, 0, 0, kMaxTrackLen), 0); +} + +// ---------------------------------------------------------------- problem 4 -- + +TEST(ValidatedLength, RejectsAZeroLengthTrack) +{ + // insert_disk() divides the rotation speed by the track length, so a track + // declared zero bytes long used to reach a division by zero. + EXPECT_EQ(gcr_validated_track_length(0x0000, 0x2000, kMaxSize, kMaxTrackLen), 0); + EXPECT_EQ(gcr_validated_track_length(0x8000, 0x2000, kMaxSize, kMaxTrackLen), 0); +} + +// ------------------------------------------------------- the header itself -- + +TEST(HeaderReadable, RejectsTheAbsentTrackMarker) +{ + EXPECT_FALSE(gcr_track_header_is_readable(0, 4096)); +} + +TEST(HeaderReadable, AcceptsAHeaderWhollyInsideWhatWasRead) +{ + EXPECT_TRUE(gcr_track_header_is_readable(4094, 4096)); +} + +TEST(HeaderReadable, RejectsAHeaderStraddlingTheEndOfWhatWasRead) +{ + // One byte of the length word is in the file, the other is stale buffer. + EXPECT_FALSE(gcr_track_header_is_readable(4095, 4096)); + EXPECT_FALSE(gcr_track_header_is_readable(4096, 4096)); +} + +TEST(HeaderReadable, RejectsAPointerPastWhatWasRead) +{ + EXPECT_FALSE(gcr_track_header_is_readable(8192, 4096)); + EXPECT_FALSE(gcr_track_header_is_readable(0xFFFFFFFFu, 4096)); +} + +// ------------------------------------------------------------- problems 1+3 -- + +TEST(TrackParameters, ProgramsARealTrackFromItsOwnLength) +{ + uint32_t address = 0, param = 0; + gcr_track_parameters(0x00800000, 0x1E0C, 0x00900000, kDummyLen, kRotationSpeed, + &address, ¶m); + EXPECT_EQ(address, 0x00800000u); + EXPECT_EQ(param & 0xFFFFu, 0x1E0Bu); // last valid offset + EXPECT_EQ(param >> 16, kRotationSpeed / 0x1E0C); // bit time +} + +TEST(TrackParameters, GivesAnAbsentTrackTheDummyAndASafeLength) +{ + // Problem 1: before the fix the else branch read two locals that the if + // branch had never written, so the first track of an image whose tracks + // are all absent programmed dummy_track with whatever was on the stack. + uint32_t address = 0, param = 0; + gcr_track_parameters(0, 0, 0x00900000, kDummyLen, kRotationSpeed, &address, ¶m); + EXPECT_EQ(address, 0x00900000u); + EXPECT_EQ(param & 0xFFFFu, (uint32_t)GCR_EMPTY_TRACK_PARAM); + EXPECT_EQ(param >> 16, kRotationSpeed / (uint32_t)kDummyLen); +} + +TEST(TrackParameters, DoesNotCarryTheNeighbourLengthIntoAnAbsentTrack) +{ + // Problem 3: add_blank_tracks() fills only even indices, so every odd one + // takes the absent path on every image. It used to be handed the dummy + // track's address together with the length left behind by the preceding + // track, a pairing that is safe only because the longest entry in + // track_lengths[] happens to equal GCRIMAGE_DUMMYTRACKLEN exactly. + // + // What has to hold is that the absent case does not depend on what came + // before it. So: program a long real track, then an absent one, and + // require the same answer as an absent track programmed on its own. + uint32_t alone_address = 0, alone_param = 0; + gcr_track_parameters(0, 0, 0x00900000, kDummyLen, kRotationSpeed, + &alone_address, &alone_param); + + uint32_t address = 0, param = 0; + gcr_track_parameters(0x00800000, kMaxTrackLen, 0x00900000, kDummyLen, kRotationSpeed, + &address, ¶m); + gcr_track_parameters(0, 0, 0x00900000, kDummyLen, kRotationSpeed, &address, ¶m); + + EXPECT_EQ(address, alone_address); + EXPECT_EQ(param, alone_param); + EXPECT_TRUE((int)((param & 0xFFFFu) + 1) <= kDummyLen); +} + +TEST(TrackParameters, TreatsAZeroLengthTrackAsAbsent) +{ + // Problem 4 again, one layer down: an address with a zero length must not + // reach the division either. + uint32_t address = 0, param = 0; + gcr_track_parameters(0x00800000, 0, 0x00900000, kDummyLen, kRotationSpeed, + &address, ¶m); + EXPECT_EQ(address, 0x00900000u); + EXPECT_EQ(param & 0xFFFFu, (uint32_t)GCR_EMPTY_TRACK_PARAM); +} + +TEST(TrackParameters, TreatsANegativeLengthAsAbsent) +{ + uint32_t address = 0, param = 0; + gcr_track_parameters(0x00800000, -1, 0x00900000, kDummyLen, kRotationSpeed, + &address, ¶m); + EXPECT_EQ(address, 0x00900000u); +} + +// --------------------------------------------------------- the MFM header -- +// +// Reported by @chrisgleissner against the second version of this fix. The +// length check above proves that the bytes a track declares were delivered. +// map_gcr_image_to_mfm() then makes a second and larger assumption about the +// same track: that it is long enough to carry the MFM metadata area, which it +// reads whole and reserves the remainder behind. + +TEST(MfmHeader, RejectsATrackTooShortToCarryTheMetadata) +{ + EXPECT_FALSE(gcr_track_can_hold_mfm_header(1, kMfmHeaderSize)); + EXPECT_FALSE(gcr_track_can_hold_mfm_header(kMfmHeaderSize - 1, kMfmHeaderSize)); +} + +TEST(MfmHeader, AcceptsATrackExactlyAsLongAsTheMetadata) +{ + EXPECT_TRUE(gcr_track_can_hold_mfm_header(kMfmHeaderSize, kMfmHeaderSize)); + EXPECT_EQ(gcr_mfm_reserved_space(kMfmHeaderSize, kMfmHeaderSize), 0u); +} + +TEST(MfmHeader, ReservesWhatLiesBehindTheMetadata) +{ + EXPECT_EQ(gcr_mfm_reserved_space(0x1E0C, kMfmHeaderSize), (uint32_t)(0x1E0C - 162)); + EXPECT_EQ(gcr_mfm_reserved_space(1000, kMfmHeaderSize), 838u); +} + +TEST(MfmHeader, NeverUnderflowsTheReservedSpace) +{ + // The field is a uint32_t and gates every track write. A short track used + // to reach it as a subtraction, so "1 - 162" became 4294967135 and + // UpdateTrack() admitted a write of any size at all. + EXPECT_EQ(gcr_mfm_reserved_space(1, kMfmHeaderSize), 0u); + EXPECT_EQ(gcr_mfm_reserved_space(0, kMfmHeaderSize), 0u); + EXPECT_EQ(gcr_mfm_reserved_space(-1, kMfmHeaderSize), 0u); + for (int len = -4; len < kMfmHeaderSize; len++) { + EXPECT_EQ(gcr_mfm_reserved_space(len, kMfmHeaderSize), 0u); + } +} + +TEST(MfmHeader, TheReportedEndOfBufferCase) +{ + // @chrisgleissner's file, exactly: GCRIMAGE_MAXSIZE bytes long, track 0 + // pointing at GCRIMAGE_MAXSIZE - 3, declaring 0x8001 -- one byte of track, + // marked MFM -- with a sector count of 32 in the final byte of the file. + const uint32_t offset = kMaxSize - 3; + const uint16_t declared = 0x8001; + + // The header is readable and the one declared byte really was delivered, + // so the length check passes it. That is correct and not the bug. + EXPECT_TRUE(gcr_track_header_is_readable(offset, kMaxSize)); + EXPECT_EQ(gcr_validated_track_length(declared, offset, kMaxSize, kMaxTrackLen), 1); + + // The mapping is where it has to stop. Believing the marker would read the + // full 162-byte metadata area from a track holding one byte at the very end + // of the buffer, so the sector loop runs past the allocation. + const int length = gcr_validated_track_length(declared, offset, kMaxSize, kMaxTrackLen); + EXPECT_FALSE(gcr_track_can_hold_mfm_header(length, kMfmHeaderSize)); + EXPECT_EQ(gcr_mfm_reserved_space(length, kMfmHeaderSize), 0u); +} + +TEST(MfmHeader, TreatsAnAbsurdHeaderSizeAsNoHeader) +{ + EXPECT_FALSE(gcr_track_can_hold_mfm_header(0x1E0C, 0)); + EXPECT_FALSE(gcr_track_can_hold_mfm_header(0x1E0C, -162)); + EXPECT_EQ(gcr_mfm_reserved_space(0x1E0C, 0), 0u); +} diff --git a/tests/e2e/drive/g71_block_reader.asm b/tests/e2e/drive/g71_block_reader.asm new file mode 100644 index 000000000..65ac09b49 --- /dev/null +++ b/tests/e2e/drive/g71_block_reader.asm @@ -0,0 +1,177 @@ +; Read one block through the drive's U1 command and hand it to the host. +; +; U1 is used on purpose: it is served by the DOS of a 1541, a 1571 and a 1581 +; alike, so this reader needs no ROM entry point and no job queue, and it does +; not change when the drive model does. The host supplies DEVICE, TRACK and +; SECTOR and checks the copied block at RESULT_DATA. + +SETLFS = $ffba +SETNAM = $ffbd +OPEN = $ffc0 +CLOSE = $ffc3 +CHKIN = $ffc6 +CHKOUT = $ffc9 +CLRCHN = $ffcc +CHRIN = $ffcf +CHROUT = $ffd2 +READST = $ffb7 + +RESULT_STATUS = $c000 +RESULT_READY = $c001 +RESULT_DOS = $c002 +RESULT_IO = $c003 +RESULT_DATA = $c100 + +STATUS_RUNNING = $00 +STATUS_DONE = $01 +STATUS_IO_ERROR = $02 +READY_MARK = $a5 + +* = $0801 + .word basic_end, 2026 + .null $9e, format("%d", start) +basic_end: + .word 0 + +start: + lda #STATUS_RUNNING + sta RESULT_STATUS + lda #READY_MARK + sta RESULT_READY + lda #0 + sta RESULT_DOS + sta RESULT_IO + + ; command channel + lda #15 + ldx #DEVICE + ldy #15 + jsr SETLFS + lda #0 + ldx #0 + ldy #0 + jsr SETNAM + jsr OPEN + bcs io_error + + ; initialise first: it frees every buffer, so a "#" opened before it is gone + jsr init_drive + + ; buffer channel on "#" + lda #2 + ldx #DEVICE + ldy #2 + jsr SETLFS + lda #1 + ldx #buffer_name + jsr SETNAM + jsr OPEN + bcs io_error + + jsr send_u1 + jsr read_block + jsr read_error_channel + + lda #STATUS_DONE +finish: + sta RESULT_STATUS + lda #2 + jsr CLOSE + lda #15 + jsr CLOSE + jsr CLRCHN + rts + +io_error: + jsr READST + sta RESULT_IO + lda #STATUS_IO_ERROR + bne finish + +; The DOS caches the disk ID and refuses a block whose header carries another +; one. An initialise makes it re-read the BAM, so a freshly mounted image is +; read on its own terms rather than on the previous mount's. +init_drive: + ldx #15 + jsr CHKOUT + bcc + + jmp io_error ++ + ldy #0 +- lda init_command,y + jsr CHROUT + iny + cpy #init_command_end-init_command + bne - + lda #13 + jsr CHROUT + jsr CLRCHN + rts + +send_u1: + ldx #15 + jsr CHKOUT + bcc + + jmp io_error ++ + ldy #0 +- lda u1_command,y + jsr CHROUT + iny + cpy #u1_command_end-u1_command + bne - + lda #13 + jsr CHROUT + jsr CLRCHN + rts + +read_block: + ldx #2 + jsr CHKIN + bcc + + jmp io_error ++ + ldy #0 +- jsr CHRIN + sta RESULT_DATA,y + iny + bne - + jsr CLRCHN + rts + +; The first byte of the DOS answer is the tens digit of the error number, the +; second the units. Both are kept so that a failure says which error it was +; rather than only that there was one. +read_error_channel: + ldx #15 + jsr CHKIN + bcc + + jmp io_error ++ + jsr CHRIN + and #$0f + asl + asl + asl + asl + sta RESULT_DOS + jsr CHRIN + and #$0f + ora RESULT_DOS + sta RESULT_DOS + jsr CLRCHN + rts + +buffer_name: + .text "#" + +; Upper case: 64tass passes .text through as ASCII, and the DOS knows its +; commands only as the PETSCII $49 "I" and $55 "U"; "i0" is error 31. +init_command: + .text "I0" +init_command_end: + +u1_command: + .text "U1 2 0 ", format("%d", TRACK), " ", format("%d", SECTOR) +u1_command_end: diff --git a/tests/e2e/drive/g71_mfm_bounds_test.py b/tests/e2e/drive/g71_mfm_bounds_test.py new file mode 100644 index 000000000..1d0998ada --- /dev/null +++ b/tests/e2e/drive/g71_mfm_bounds_test.py @@ -0,0 +1,374 @@ +#!/usr/bin/env python3 +"""E2E: a G71 whose MFM-marked tracks are malformed must not take the drive +with it, and the GCR side of the same disk has to keep reading. + +The fixtures are built here rather than shipped, because what is interesting +about them is exactly the two-byte length word in front of each track, and a +committed binary would hide it. Three images, one normal and two malformed in +the ways the bounds checks in `gcr_track_bounds.h` are about: + + mixed a well-formed disk: GCR tracks, and one track carrying the MFM + payload this firmware writes -- sector count, version byte, five + bytes per sector, sector data from offset 162 on. + shortmfm the same disk, but the MFM-marked track declares one single byte. + The metadata area alone is 162, so there is nothing to read; the + mapping used to read it anyway. + overlong the same disk, but one track declares 0x4123. Read as fifteen + bits that is 16675, above GCRIMAGE_MAXTRACKLEN, and the track has + to be refused. Masked to fourteen it became 291 and was mounted. + +Every case asserts the same two things: the device is still reachable, and +track 18 sector 0 still comes back through the drive's own U1 command. A +malformed track elsewhere on the disk may not cost the rest of it. +""" + +import argparse +import posixpath +import sys +import time +from pathlib import Path + +SCRIPT_DIR = Path(__file__).resolve().parent + +# The one stanza that puts the shared library on sys.path; see tests/lib/bootstrap.py. +sys.path.insert(0, str(next(p for p in Path(__file__).resolve().parents + if (p / "tests" / "lib").is_dir()) / "tests" / "lib")) +import bootstrap # noqa: E402,F401 +import cli # noqa: E402 + +import ftp as ftp_lib # noqa: E402 +from api import DriveInfo, UltimateApi # noqa: E402 +from assembler import assemble # noqa: E402 +from report import ( # noqa: E402 + Failure, + check, + detail, + format_exception, + section, + suite_fail, + suite_ok, +) + +SUITE = "g71_mfm_bounds_test" +SOURCE = SCRIPT_DIR / "g71_block_reader.asm" + +# ---------------------------------------------------------------- GCR encoding + +# The 1541's 4-to-5 code. Its point is that no legal sequence has more than two +# zero bits in a row, which is what makes a run of zeros recognisable as an +# unformatted or weak area rather than as data. +GCR_NIBBLE = ( + 0x0a, 0x0b, 0x12, 0x13, 0x0e, 0x0f, 0x16, 0x17, + 0x09, 0x19, 0x1a, 0x1b, 0x0d, 0x1d, 0x1e, 0x15, +) + +SYNC = b"\xff" * 5 +GAP = b"\x55" + +# Sectors per track, and the nominal track length of each speed zone. +ZONES = ((31, 0, 17, 6250), (25, 1, 18, 6666), (18, 2, 19, 7142), (1, 3, 21, 7692)) + +DISK_ID = b"E2" + + +def zone_of(track: int) -> tuple: + """Returns (speed zone, sectors, nominal length) for a 1541 track.""" + for first, zone, sectors, length in ZONES: + if track >= first: + return zone, sectors, length + raise ValueError(track) + + +def gcr_encode(data: bytes) -> bytes: + """Four bytes in, five out: eight nibbles of five bits each.""" + if len(data) % 4: + raise ValueError("GCR encodes whole groups of four bytes") + out = bytearray() + for i in range(0, len(data), 4): + bits = 0 + for byte in data[i:i + 4]: + bits = (bits << 5) | GCR_NIBBLE[byte >> 4] + bits = (bits << 5) | GCR_NIBBLE[byte & 0x0f] + out += bits.to_bytes(5, "big") + return bytes(out) + + +def sector_image(track: int, sector: int, payload: bytes) -> bytes: + """One sector as it lies on the medium: header, gap, data, gap.""" + if len(payload) != 256: + raise ValueError("a sector holds 256 bytes") + id1, id2 = DISK_ID[0], DISK_ID[1] + header = bytes((0x08, sector ^ track ^ id2 ^ id1, sector, track, id2, id1, 0x0f, 0x0f)) + checksum = 0 + for byte in payload: + checksum ^= byte + block = bytes((0x07,)) + payload + bytes((checksum, 0x00, 0x00)) + return (SYNC + gcr_encode(header) + GAP * 9 + + SYNC + gcr_encode(block) + GAP * 9) + + +def gcr_track(track: int, payload_of) -> bytes: + """A whole GCR track, padded to its zone's nominal length.""" + _, sectors, length = zone_of(track) + data = b"".join(sector_image(track, s, payload_of(track, s)) for s in range(sectors)) + if len(data) > length: + raise ValueError(f"track {track} came out at {len(data)}, zone allows {length}") + return data + GAP * (length - len(data)) + + +# --------------------------------------------------------------- image layout + +SIGNATURE = b"GCR-1571" +HALF_TRACKS = 168 # 84 per side, the 1571 in double-sided mode +FIRST_SIDE_1 = 84 +MAX_TRACK_SIZE = 7928 # the usual value; the header is what actually binds +SLOT = MAX_TRACK_SIZE + 2 # length word plus the track it announces +TABLES = 12 + HALF_TRACKS * 4 * 2 + +DOS_TRACK = 18 +SENTINEL_SECTOR = 1 +SENTINEL = bytes((value ^ 0xa5) for value in range(256)) + +# Physical track 40 on side 0. Outside what the DOS ever touches, so a broken +# one cannot be confused with a broken directory. +MFM_HALF_TRACK = (40 - 1) * 2 +OVERLONG_HALF_TRACK = (39 - 1) * 2 + +# The payload this firmware writes for an MFM track: a sector count, a version +# byte of zero, and five bytes for each of up to 32 sectors -- track, side, +# sector, size code, error byte. Sector data starts behind that fixed area, +# whatever the sector count is (c1541.cc, mfm_update_callback). +MFM_MAX_SECTORS = 32 +MFM_HEADER_SIZE = 2 + MFM_MAX_SECTORS * 5 +MFM_SECTORS = 10 +MFM_SIZE_CODE = 2 # 1 << (7 + 2) = 512 bytes +MFM_SECTOR_BYTES = 1 << (7 + MFM_SIZE_CODE) + +MFM_MARKER = 0x8000 + + +def bam() -> bytes: + """Track 18 sector 0. Only the fields the DOS reads to learn the disk.""" + block = bytearray(256) + block[0:2] = bytes((DOS_TRACK, 1)) + block[2] = 0x41 # 'A', DOS version + for track in range(1, 36): + _, sectors, _ = zone_of(track) + entry = 4 + (track - 1) * 4 + block[entry] = 0 if track == DOS_TRACK else sectors + block[entry + 1:entry + 4] = b"\xff\xff\x1f" + block[0x90:0xa2] = b"\xa0" * 18 + block[0x90:0x99] = b"MFM BOUNDS"[:9] + block[0xa2:0xa4] = DISK_ID + block[0xa4] = 0xa0 + block[0xa5:0xa7] = b"2A" + block[0xa7:0xab] = b"\xa0" * 4 + return bytes(block) + + +def sector_payload(track: int, sector: int) -> bytes: + if track == DOS_TRACK and sector == 0: + return bam() + if track == DOS_TRACK and sector == SENTINEL_SECTOR: + return SENTINEL + return bytes((track, sector)) + bytes(254) + + +def mfm_track(sectors: int = MFM_SECTORS) -> bytes: + """A well-formed MFM payload, laid out as the firmware lays it out.""" + header = bytearray(MFM_HEADER_SIZE) + header[0] = sectors + header[1] = 0 # version + for index in range(sectors): + entry = 2 + index * 5 + header[entry:entry + 5] = bytes((40, 0, index + 1, MFM_SIZE_CODE, 0)) + data = bytearray() + for index in range(sectors): + data += bytes((index + 1,)) * MFM_SECTOR_BYTES + return bytes(header) + bytes(data) + + +def g71(tracks: dict) -> bytes: + """`tracks` maps a half-track index to (declared word, track bytes). + + Tracks that fit the header's maximum go into fixed slots behind the + tables, the way a G64 is normally laid out. One that does not is appended + behind them, which the format allows outright: "The location of the actual + track or speed zone data is not important." + """ + image = bytearray(TABLES) + image[0:8] = SIGNATURE + image[8] = 0 # G64 version + image[9] = HALF_TRACKS + image[10:12] = MAX_TRACK_SIZE.to_bytes(2, "little") + + offsets = {} + inline = sorted(index for index, (_, data) in tracks.items() + if len(data) <= MAX_TRACK_SIZE) + for slot, index in enumerate(inline): + offsets[index] = TABLES + slot * SLOT + image += bytearray(len(inline) * SLOT) + + for index in sorted(tracks): + if index not in offsets: + offsets[index] = len(image) + image += bytearray(2 + len(tracks[index][1])) + + for index, (declared, data) in tracks.items(): + at = offsets[index] + image[at:at + 2] = declared.to_bytes(2, "little") + image[at + 2:at + 2 + len(data)] = data + + speeds = 12 + HALF_TRACKS * 4 + for index in tracks: + at = 12 + index * 4 + image[at:at + 4] = offsets[index].to_bytes(4, "little") + track = index // 2 + 1 + zone = zone_of(track)[0] if index % 2 == 0 and track <= 35 else 0 + at = speeds + index * 4 + image[at:at + 4] = zone.to_bytes(4, "little") + return bytes(image) + + +def fixtures() -> dict: + """The three images, each built from the same GCR side 0.""" + gcr = {(track - 1) * 2: gcr_track(track, sector_payload) for track in range(1, 36)} + common = {index: (len(data), data) for index, data in gcr.items()} + + payload = mfm_track() + well_formed = dict(common) + well_formed[MFM_HALF_TRACK] = (MFM_MARKER | len(payload), payload) + + short = dict(common) + short[MFM_HALF_TRACK] = (MFM_MARKER | 1, b"\x00") + + overlong = dict(common) + overlong[OVERLONG_HALF_TRACK] = (0x4123, bytes(0x4123 & 0x7fff)) + + return {"mixed": well_formed, "shortmfm": short, "overlong": overlong} + + +CASES = ("mixed", "shortmfm", "overlong") + +RESULT_STATUS = 0xc000 +RESULT_BYTES = 4 +RESULT_DATA = 0xc100 +STATUS_DONE = 0x01 +READY_MARK = 0xa5 +PROGRAM_TIMEOUT_SECONDS = 60.0 +POLL_SECONDS = 0.5 +LIVENESS_TIMEOUT_SECONDS = 20.0 + + +def mounted_path(drive: DriveInfo) -> str: + if drive.image_file.startswith("/"): + return drive.image_file + return posixpath.join(drive.image_path, drive.image_file) if drive.image_file else "" + + +class SuiteRunner: + def __init__(self, args: argparse.Namespace) -> None: + self.args = args + self.api = UltimateApi(args.host, args.password or None, args.timeout) + self.slot = "" + self.original: DriveInfo | None = None + self.paths: dict[str, str] = {} + + def prepare(self) -> bytes: + drives = self.api.drives.list() + self.slot = "b" if "b" in drives else "a" + if self.slot not in drives: + raise Failure("the device exposes neither drive a nor drive b") + self.original = drives[self.slot] + if self.original.bus_id is None: + raise Failure(f"drive {self.slot} has no IEC bus ID") + + self.api.drives.set_mode(self.slot, "1571") + self.api.drives.on(self.slot) + images = fixtures() + with ftp_lib.session(self.args.host, self.args.password or None) as client: + for name in CASES: + path = f"/Temp/g71-mfm-{name}.g71" + ftp_lib.store(client, path, g71(images[name])) + self.paths[name] = path + return assemble(SOURCE, {"DEVICE": self.original.bus_id, + "TRACK": DOS_TRACK, "SECTOR": SENTINEL_SECTOR}) + + def await_program(self) -> bytes: + deadline = time.monotonic() + PROGRAM_TIMEOUT_SECONDS + while True: + result = self.api.machine.readmem(RESULT_STATUS, RESULT_BYTES) + if result[1] == READY_MARK and result[0] != 0: + return result + if time.monotonic() >= deadline: + raise Failure(f"block reader did not finish: result={result.hex(' ')}") + time.sleep(POLL_SECONDS) + + def run_case(self, name: str, prg: bytes) -> None: + path = self.paths[name] + section(name) + + with check(f"mount the {name} G71 on drive {self.slot}"): + self.api.drives.mount(self.slot, path, type="g71", mode="readonly") + mounted = self.api.drives.get(self.slot) + if posixpath.basename(path) not in mounted.image_file: + raise Failure(f"drive reports {mounted.image_file!r}, expected {path!r}") + + with check("device remains reachable after mount"): + reason = self.api.unreachable_reason(LIVENESS_TIMEOUT_SECONDS) + if reason: + raise Failure(reason) + + with check(f"track {DOS_TRACK} sector {SENTINEL_SECTOR} still reads"): + self.api.machine.writemem(RESULT_STATUS, bytes(RESULT_BYTES), idempotent=True) + self.api.machine.writemem(RESULT_DATA, bytes(256), idempotent=True) + status, _, body = self.api.runners.upload("run_prg", prg) + if status != 200: + raise Failure(f"run_prg returned HTTP {status}: {body[:160]!r}") + result = self.await_program() + data = self.api.machine.readmem(RESULT_DATA, 256) + detail(f"dos=${result[2]:02x}, io=${result[3]:02x}") + if result[0] != STATUS_DONE: + raise Failure(f"reader failed: result={result.hex(' ')}") + if data != SENTINEL: + raise Failure(f"sentinel came back as {data[:8].hex(' ')}...") + + def cleanup(self) -> None: + try: + if self.slot: + self.api.drives.remove(self.slot) + except Exception: + pass + try: + with ftp_lib.session(self.args.host, self.args.password or None) as client: + for path in self.paths.values(): + try: + ftp_lib.delete(client, path) + except Exception: + pass + except Exception: + pass + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + cli.add_device_arguments(parser, timeout=15.0, colour=False) + args = parser.parse_args() + runner = SuiteRunner(args) + try: + prg = runner.prepare() + for name in CASES: + runner.run_case(name, prg) + except Failure as failure: + suite_fail(SUITE, str(failure)) + return 1 + except Exception as exc: # noqa: BLE001 - the harness reports, it does not raise + suite_fail(SUITE, format_exception(exc)) + return 1 + finally: + runner.cleanup() + return suite_ok(SUITE) + + +if __name__ == "__main__": + sys.exit(main())