From ce0f18f4ef868503976cb44cd33bb99ed6e7a5b0 Mon Sep 17 00:00:00 2001 From: Anachronaut Date: Tue, 25 Aug 2026 23:47:02 -0400 Subject: [PATCH] Refuse a directory whose last entries cannot be named as a parent A parent is an entry index PLUS ONE in two bytes, so entry 65535 has no parent number: adding one wraps to zero, and zero is the root. Eight entries to a block, so 8192 directory blocks reaches it and SplitDisk formatted that happily. It does not fail by refusing, which is why it was worth chasing rather than reasoning about. Reproduced on a disk built for it: mkdir /deep/child, with /deep at entry 65535, printed 'Made "/deep/child" as entry 0' and put child in the ROOT. Listing /deep then showed nothing, because the search is for a parent of 65536 and the entry carries zero - so the same mkdir succeeded again, and again, and five entries called /child piled up 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 one per attempt. 8191 blocks is the most, giving 65528 entries. Refused when formatting and again when reading, in both implementations, because a disk claiming more was made by something that never checked. On the machine only the high byte of the count has to be looked at: anything from 0x20 up is too many. Three checks, all of which fail with their guard removed. The machine's disk claims the size rather than having it, so the test image is 64 blocks that lie rather than sixteen megabytes that do not - mounting is refused at the geometry, which is read out of block 0. --- Programs/CosmOS/README.md | 17 +++++++++++++++++ Programs/CosmOS/Source/sbfs.asm | 28 ++++++++++++++++++++++++++++ Source/DiskTool/SplitDisk.c | 20 +++++++++++++++++++- Source/DiskTool/sbfs.h | 21 +++++++++++++++++++++ Tests/disk.sh | 17 +++++++++++++++++ Tests/expected/cosmosBigDir.out | 6 ++++++ Tests/input/cosmosBigDir.in | 2 ++ Tests/makedisks.sh | 11 +++++++++++ Tests/manifest | 8 ++++++++ 9 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 Tests/expected/cosmosBigDir.out create mode 100644 Tests/input/cosmosBigDir.in 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.