diff --git a/Programs/CosmOS/README.md b/Programs/CosmOS/README.md index 045cc37..0fc426f 100644 --- a/Programs/CosmOS/README.md +++ b/Programs/CosmOS/README.md @@ -237,6 +237,23 @@ because it is the only thing that makes the difference between them real. A disk readable by anything that has never heard of a directory right up until it actually has one. +**A disk may have at most 8,191 directory blocks**, which is 65,528 entries, and that +number comes from the parent field rather than from anything about size. A parent is an +index *plus one* in two bytes, so entry 65,535 has no parent number at all: adding one +wraps to zero, and zero means the root. + +The failure is worth describing, because it is the shape of failure this format has to +watch for. Such an entry does not refuse what is put inside it. It writes a parent of zero +and the thing lands in **the root**, while whatever asked is told it went where it asked +for. Looking in that directory afterwards finds nothing, because the search is for a +parent the entry does not carry - so the same create succeeds again, and again, filling +the root with entries of one name. Two entries of one name in one directory is precisely +what `rename` refuses on the grounds that a search answers with whichever it meets first +and the rest can never be reached again; this made them by the handful, one per attempt. + +Both implementations refuse to format past the bound, and refuse to read a disk that +claims it - because a disk claiming it was made by something that never checked. + Four things are refused, and each refusal is the reason a separate command exists: **`rmdir` will not take a file and `delete` will not take a directory.** Neither can be diff --git a/Programs/CosmOS/Source/sbfs.asm b/Programs/CosmOS/Source/sbfs.asm index 8240e6f..30f6ffe 100644 --- a/Programs/CosmOS/Source/sbfs.asm +++ b/Programs/CosmOS/Source/sbfs.asm @@ -98,6 +98,27 @@ sbfsGeometry: DPUP.0 0d10 SETD.1 SbfsDirBlocks CALL sbfsCopyWord + + ; A DIRECTORY WHOSE LAST ENTRIES CANNOT BE NAMED AS A PARENT. Eight entries to a block, + ; and the parent is an index plus one in two bytes, so entry 65535 has no parent number + ; at all - adding one wraps to zero, and zero is the root. + ; + ; It does not fail by refusing. Anything created inside such a directory writes a parent + ; of zero and lands in the ROOT, while whatever asked reports the path it wanted; + ; looking there afterwards finds nothing, because the search is for a parent the entry + ; does not carry, so the same create succeeds over and over and piles up entries of one + ; name in the root. Two entries of one name in one place is the thing rename refuses + ; outright, and this made them by the handful. + ; + ; 8191 blocks is the most, so anything from 0x2000 up is refused. Only the high byte has + ; to be looked at to know. + SETD.0 SbfsDirBlocks + LDA.0 + INIB 0x20 + CCF + SUB + BNC sbfsMountTooBig ; The high byte is 0x20 or more, so more than 8191 blocks. + SETD.0 SbfsBuffer DPUP.0 0d06 SETD.1 SbfsDiskBlocks @@ -108,6 +129,13 @@ sbfsGeometry: ADD ; Q is zero: mounted. RET +sbfsMountTooBig: + RSTA + INIB 0d1 + CCF + ADD ; Q is not zero: not a disk this will mount. + RET + ; ---- Finding something by path ---- ; ; DP0 points at a path ending in a zero byte: names with '/' between them. A path that diff --git a/Source/DiskTool/SplitDisk.c b/Source/DiskTool/SplitDisk.c index 1172a1f..592d5ff 100644 --- a/Source/DiskTool/SplitDisk.c +++ b/Source/DiskTool/SplitDisk.c @@ -78,10 +78,20 @@ static int readSuperblock(FILE *image, Superblock *super) { block[SBFS_SUPER_VERSION], SBFS_VERSION_FLAT, SBFS_VERSION_TREE); return 1; } + // A directory big enough that its last entries cannot be named as a parent. See + // sbfs.h: those entries do not refuse what is put in them, they quietly put it in the + // root instead. Refused on the way in, so that nothing below ever has to wonder. + uint16_t directoryBlocks = readWord(block + SBFS_SUPER_DIRBLOCKS); + if (directoryBlocks > SBFS_MAX_DIRECTORY_BLOCKS) { + fprintf(stderr, "Error: That disk claims %u directory blocks, and %u is the most" + " that leaves every entry able to be named as a parent.\n", + directoryBlocks, SBFS_MAX_DIRECTORY_BLOCKS); + return 1; + } super->version = block[SBFS_SUPER_VERSION]; super->diskBlocks = readWord(block + SBFS_SUPER_DISK); super->directoryStart = readWord(block + SBFS_SUPER_DIRSTART); - super->directoryBlocks = readWord(block + SBFS_SUPER_DIRBLOCKS); + super->directoryBlocks = directoryBlocks; super->freeBlocks = readWord(block + SBFS_SUPER_FREE); return 0; } @@ -397,6 +407,14 @@ static uint16_t countFree(const Directory *directory, const Superblock *super) { // ---- Commands ---- static int commandFormat(const char *path, uint16_t blocks, uint16_t directoryBlocks) { + if (directoryBlocks > SBFS_MAX_DIRECTORY_BLOCKS) { + fprintf(stderr, "Error: %u directory blocks is %u entries, and entry 65535 has no" + " parent number - adding one wraps to zero, which is the root." + " %u blocks is the most, giving %u entries.\n", + directoryBlocks, (unsigned)directoryBlocks * SBFS_ENTRIES_PER_BLOCK, + SBFS_MAX_DIRECTORY_BLOCKS, SBFS_MAX_ENTRIES); + return 1; + } if (blocks <= 1u + directoryBlocks) { fprintf(stderr, "Error: A disk of %u blocks has no room for a superblock and a" " directory of %u.\n", blocks, directoryBlocks); diff --git a/Source/DiskTool/sbfs.h b/Source/DiskTool/sbfs.h index 33e03e3..b5ba5ba 100644 --- a/Source/DiskTool/sbfs.h +++ b/Source/DiskTool/sbfs.h @@ -119,6 +119,27 @@ #define SBFS_PARENT_OF(index) ((uint16_t)((index) + 1)) #define SBFS_PARENT_INDEX(parent) ((int)(parent) - 1) +// ---- How big a directory may be ---- +// +// The parent is an index plus one in sixteen bits, so index 65535 has no representation: +// adding one wraps to zero, and zero is the root. An entry that cannot be named as a +// parent is a directory that cannot hold anything, and it does not fail by refusing. +// +// WHAT IT DOES INSTEAD IS WORSE THAN FAILING. Creating something inside it writes a +// parent of zero, so the thing lands in the root while the tool reports the path it was +// asked for. Looking in that directory afterwards finds nothing, because the search is +// for a parent of 65536 and the entry says zero - so the same create succeeds again, and +// again, piling up entries of one name in the root. Duplicate names in one directory are +// the one thing rename refuses outright, on the grounds that a search answers with +// whichever it meets first and the rest can never be reached; this manufactured them. +// +// Eight entries to a block, and 65535 entries is the most that leaves every index one +// short of the wrap. 8191 blocks gives 65528 of them, which is the last whole block that +// fits. Checked when formatting and again when reading, because a disk claiming more may +// have been made by something that never checked at all. +#define SBFS_MAX_DIRECTORY_BLOCKS 8191 +#define SBFS_MAX_ENTRIES (SBFS_MAX_DIRECTORY_BLOCKS * SBFS_ENTRIES_PER_BLOCK) + // Paths are separated by this, and a leading one means "from the root". A name may not // contain it, which is what makes a path unambiguous without any quoting. #define SBFS_SEPARATOR '/' diff --git a/Tests/disk.sh b/Tests/disk.sh index 4d2a7ec..8a9bb33 100755 --- a/Tests/disk.sh +++ b/Tests/disk.sh @@ -155,6 +155,23 @@ before=$(blocksFree tree.img) "$TOOL" mkdir tree.img /Empty >/dev/null 2>&1 check "a directory costs no blocks" [ "$before" = "$(blocksFree tree.img)" ] +# ---- A directory no bigger than the parent field can name ---- +# +# Eight entries to a block and the parent is an index plus one in two bytes, so entry +# 65535 has no parent number: adding one wraps to zero, and zero is the root. Such an +# entry does not refuse what is put inside it. It writes the thing into the ROOT while +# reporting the path that was asked for, and then cannot find it again - so the same +# create succeeds over and over, piling up entries of one name in one directory, which is +# the exact corruption rename exists to refuse. +refuses "no directory past the wrap" "$TOOL" format huge.img 65535 8192 +check "the largest that fits" "$TOOL" format huge.img 65535 8191 + +# And a disk claiming one, which is what something that never checked would have written. +# The claim is in the superblock, so it does not need a disk that size to be made. +"$TOOL" format lying.img 64 2 >/dev/null +printf '\x20\x00' | dd of=lying.img bs=1 seek=10 conv=notrunc status=none +refuses "nor reading one that claims it" "$TOOL" list lying.img + echo if [ "$FAIL" -eq 0 ]; then echo "All $PASS disk tool checks passed." diff --git a/Tests/expected/cosmosBigDir.out b/Tests/expected/cosmosBigDir.out new file mode 100644 index 0000000..9de501d --- /dev/null +++ b/Tests/expected/cosmosBigDir.out @@ -0,0 +1,6 @@ +CosmOS +no filesystem on the disk +> no filesystem on the disk +> halted +Execution halted. +[exit 0] diff --git a/Tests/input/cosmosBigDir.in b/Tests/input/cosmosBigDir.in new file mode 100644 index 0000000..d4878a2 --- /dev/null +++ b/Tests/input/cosmosBigDir.in @@ -0,0 +1,2 @@ +dir +exit diff --git a/Tests/makedisks.sh b/Tests/makedisks.sh index 190d51d..4bca435 100755 --- a/Tests/makedisks.sh +++ b/Tests/makedisks.sh @@ -342,3 +342,14 @@ for i in 1 2 3 4 5 6 7 8; do DEEPPATH="$DEEPPATH/abcdefghijklmnopqrst0$i" "$TOOL" mkdir "$DISKS/deep.img" "$DEEPPATH" >/dev/null done + +# A disk whose superblock claims a directory bigger than the parent field can name. Entry +# 65535 has no parent number - index plus one wraps to zero, which is the root - so +# anything created inside it lands in the root instead while the tool reports success. +# +# THE CLAIM IS ALL THAT IS NEEDED, so this is a small disk that lies rather than a sixteen +# megabyte one that tells the truth. Mounting reads block 0 and gets as far as the +# geometry, which is where it is refused. SplitDisk will not make one of these any more, +# so the superblock is written by hand. +"$TOOL" format "$DISKS/bigdir.img" 64 2 >/dev/null +printf '\x20\x00' | dd of="$DISKS/bigdir.img" bs=1 seek=10 conv=notrunc status=none diff --git a/Tests/manifest b/Tests/manifest index 4a90cec..c9ecf6c 100644 --- a/Tests/manifest +++ b/Tests/manifest @@ -345,6 +345,14 @@ cosmosCopyCompare | CosmOS/Source/cosmos.asm | run | cosmosCop # help and cd and exit from down there rather than just looking at the prompt. A prompt # that is merely wrong is a cosmetic fault; this one was writing into other variables. cosmosDeep | CosmOS/Source/cosmos.asm | run | cosmosDeep.in | - | disks/deep.img + +# A disk claiming a directory of 8192 blocks, which is 65536 entries. The last of those is +# entry 65535, and the parent field is an index PLUS ONE in two bytes - so it wraps to +# zero, which means the root. Anything created in such a directory goes into the root +# while the caller is told it went where it asked, and looking there afterwards finds +# nothing, so the same create works again and again and fills the root with entries of one +# name. Refused at mount, which is the only place it can be refused once and for all. +cosmosBigDir | CosmOS/Source/cosmos.asm | run | cosmosBigDir.in | - | disks/bigdir.img # Reading a disk that has directories on it. The machine can walk a path at this point but # cannot make a directory, so the disk is built by the host tool and read here - which is # the two implementations checking each other rather than either checking itself.