diff --git a/Programs/CosmOS/Apps/Claim.asm b/Programs/CosmOS/Apps/Claim.asm new file mode 100644 index 0000000..bf35f5f --- /dev/null +++ b/Programs/CosmOS/Apps/Claim.asm @@ -0,0 +1,102 @@ +; Tries to commit a file bigger than the room it reserved. +; +; osFileStart sets aside an extent and osFileWrite refuses a block index outside it, so the +; obvious way to reach a neighbouring file - writing off the end - is already barred. This +; is the other way to the same place: reserve one block, write the one block, and then tell +; osFileDone the file came to two. +; +; NOTHING WOULD SAY SO IF THAT WERE ALLOWED. A directory entry is the only record of what a +; file owns, so an entry claiming a block it was never given simply owns it, and whatever +; owned it before owns it too. Both files then look perfectly well formed. +; +; Correct behaviour is a refusal, and the file left as it was. The reservation is one block +; and a tail of ten, so: +; +; two blocks and no tail more than was reserved refused +; one block and a tail exactly what was reserved allowed +; +; Written by Anachronaut + +#Include services.asm +#Program + #Base 0x4000 + +start: + ; One block, and ten bytes after it. + SETD.0 Name + SETD.3 0x00 0x01 + INIA 0d10 + SWI osFileStart + BNQ noStart + + SETD.1 Block + RSTA + RSTB + SWI osFileWrite + BNQ noWrite + SETD.1 Block + RSTA + INIB 0d1 + SWI osFileWrite + BNQ noWrite + + ; Two whole blocks, which is more than one block and a tail. + SETD.3 0x00 0x02 + RSTA + SWI osFileDone + BNQ refused + SETD.0 Allowed + SWI osPrintString + SWI osExit + +refused: + SETD.0 Refused + SWI osPrintString + + ; And the honest size, which is what was reserved. + SETD.3 0x00 0x01 + INIA 0d10 + SWI osFileDone + BNQ noHonest + SETD.0 Honest + SWI osPrintString + SWI osExit + +noHonest: + SETD.0 NoHonest + SWI osPrintString + SWI osExit +noStart: + SETD.0 NoStart + SWI osPrintString + SWI osExit +noWrite: + SETD.0 NoWrite + SWI osPrintString + SWI osExit + +#Data + #Base 0x2000 + +Name: +"claim.dat" +Block: + #Reserve 0d256 +Allowed: +"claiming more than was reserved was ALLOWED +" +Refused: +"claiming more than was reserved was refused +" +Honest: +"and the size it really came to was taken +" +NoHonest: +"the honest size was refused too +" +NoStart: +"no start +" +NoWrite: +"no write +" diff --git a/Programs/CosmOS/Assembler/Asm.asm b/Programs/CosmOS/Assembler/Asm.asm index dade95b..92ca217 100644 --- a/Programs/CosmOS/Assembler/Asm.asm +++ b/Programs/CosmOS/Assembler/Asm.asm @@ -1483,9 +1483,19 @@ checkImageRoom: ; ; MORE THAN THE FILE WILL COME TO, on purpose. How many vectors are actually installed is ; not known until the second pass has resolved every handler, and by then the file has to - ; exist to be written into. What IS known now is how many vectors were DECLARED, and no - ; more than that can be installed - so the room asked for is the whole file plus four - ; bytes for each of them and five for a marker. + ; exist to be written into. So the room asked for is the whole file plus four bytes for + ; every vector the table can hold, and five for a marker. + ; + ; THE LIMIT RATHER THAN THE COUNT SO FAR, and that distinction cost a real bug. Asking + ; for four bytes per vector DECLARED looks like a safe bound and is not: a device is + ; declared during the SECOND pass, in the line that implements it, so a program with one + ; installs a vector that was not counted when the room was measured. CosmOS reserved + ; 14,163 bytes and committed 14,167, writing four bytes past what it had been given - + ; which happened to land inside the last block it owned, and would not have if the + ; boundary had fallen four bytes earlier. + ; + ; The limit cannot go stale that way. It is what the vector table holds, so no assembly + ; can install more, whenever they are counted. ; ; Asking for too much costs nothing but a moment: osFileDone is told what it really came ; to and the difference goes back. Asking for too little would have meant writing off the @@ -1494,7 +1504,7 @@ checkImageRoom: SETD.2 ImgTotal CALL numSet SETD.0 ImgVecMost - SETD.2 VecCount + SETD.2 VecLimit CALL numSet SETD.0 ImgVecMost SETD.2 ImgVecMost diff --git a/Programs/CosmOS/Source/sbfs.asm b/Programs/CosmOS/Source/sbfs.asm index 5d57985..691530d 100644 --- a/Programs/CosmOS/Source/sbfs.asm +++ b/Programs/CosmOS/Source/sbfs.asm @@ -2474,6 +2474,61 @@ sbfsStreamDone: SETD.1 SbfsStreamNewTail STA.1 + ; ---- MORE THAN WAS RESERVED IS REFUSED ---- + ; + ; A writer may finish smaller than it asked for, which is the whole point of being told + ; the size here. It may not finish BIGGER. The blocks after a file belong to whatever + ; comes next, so an entry claiming more than was set aside for it claims somebody else's + ; - and nothing anywhere would say so, because a directory entry is the only record of + ; what a file owns. The count of free blocks would go wrong in the same breath, the + ; subtraction below running backwards past zero. + ; + ; osFileWrite already refuses a block index past the end. This is the same bound from the + ; other side, and it was missing: the index was checked because writing off the end was + ; the obvious way to reach a neighbour, and committing a larger size reaches the same + ; neighbour by simply claiming it. + ; + ; CHECKED BEFORE ANYTHING IS TOUCHED, which is why the temporary is found twice. The old + ; file is deleted a few lines down, and a refusal after that point would have destroyed + ; the thing it was protecting. + CALL sbfsStreamTemp + CALL sbfsScanFor + BNQ sbfsStreamNo + + ; What it was given, both halves of it. + SETD.0 SbfsStreamResBlocks + SETD.2 SbfsFileBlocks + CALL sbfsSetWord + SETD.0 SbfsFileTail + LDA.0 + SETD.1 SbfsStreamResTail + STA.1 + + ; And the room that came to, kept for the free count at the end. + CALL sbfsFileExtent + SETD.0 SbfsStreamSpare + SETD.2 SbfsWantBlocks + CALL sbfsSetWord + + ; THE SIZE IS COMPARED, NOT THE ROOM IT TAKES UP. Those are not the same question: one + ; block and a tail occupies exactly what two whole blocks occupy, so a file reserving the + ; first and committing the second claims no block it was not given - and still reports + ; two hundred and forty six bytes more than were ever written to it, which are whatever + ; the disk had there before. Bounding the blocks alone would have called that fine. + SETD.0 SbfsStreamResBlocks + SETD.2 SbfsStreamNewBlocks + CALL sbfsCompareWord + BRC sbfsStreamNo ; More whole blocks than it was given. + BNQ sbfsStreamFits ; Fewer, so the tail cannot matter. + SETD.0 SbfsStreamResTail + LDA.0 + SETD.2 SbfsStreamNewTail + LDB.2 + CCF + SUB + BRC sbfsStreamNo ; The same blocks, and a longer tail. +sbfsStreamFits: + ; Now, and not before, the old one goes. It may not be there at all, which is what ; writing something for the first time looks like from here. CALL sbfsStreamWhere @@ -2483,17 +2538,12 @@ sbfsStreamDone: sbfsStreamNoOld: ; And the temporary takes its name and its true size, which together are the whole of - ; what committing is. + ; what committing is. Found again, because deleting the old one read over the block it + ; lives in. CALL sbfsStreamTemp CALL sbfsScanFor BNQ sbfsStreamNo - ; How much room it was given, before the entry that says so is changed. - CALL sbfsFileExtent - SETD.0 SbfsStreamSpare - SETD.2 SbfsWantBlocks - CALL sbfsSetWord - PSHD.3 POPD.1 DPUP.1 0d06 @@ -2517,14 +2567,9 @@ sbfsStreamNoOld: CALL sbfsWriteBlock BNQ sbfsStreamNo - ; And the difference goes back, which is what it was given less what it kept. - SETD.0 SbfsFileBlocks - SETD.2 SbfsStreamNewBlocks - CALL sbfsSetWord - SETD.0 SbfsStreamNewTail - LDA.0 - SETD.1 SbfsFileTail - STA.1 + ; And the difference goes back, which is what it was given less what it kept. That can + ; no longer be negative: the check at the top refused the only case where it could. + CALL sbfsStreamSize CALL sbfsFileExtent SETD.0 SbfsStreamSpare SETD.2 SbfsWantBlocks @@ -2557,6 +2602,19 @@ sbfsStreamNoOld: ADD RET +; The size the writer says it came to, put back where the extent arithmetic reads it from. +; Said again rather than kept, because finding anything overwrites those two: they are where +; a find describes whatever it last looked at. +sbfsStreamSize: + SETD.0 SbfsFileBlocks + SETD.2 SbfsStreamNewBlocks + CALL sbfsSetWord + SETD.0 SbfsStreamNewTail + LDA.0 + SETD.1 SbfsFileTail + STA.1 + RET + ; The directory the file is going in, and the name it will end up under. Said again before ; each step, because every step goes to the disk and leaves the walk somewhere else. sbfsStreamWhere: @@ -2880,6 +2938,10 @@ SbfsStreamNewTail: 0x00 SbfsStreamSpare: 0x00 0x00 +SbfsStreamResBlocks: + 0x00 0x00 +SbfsStreamResTail: + 0x00 SbfsStreamAt: 0x00 0x00 SbfsStreamParent: diff --git a/Tests/expected/cosmosClaim.out b/Tests/expected/cosmosClaim.out new file mode 100644 index 0000000..86f2f7a --- /dev/null +++ b/Tests/expected/cosmosClaim.out @@ -0,0 +1,10 @@ +CosmOS +> claiming more than was reserved was refused +and the size it really came to was taken +finished +> Claim.sbx 572 +claim.dat 266 +2 files +> halted +Execution halted. +[exit 0] diff --git a/Tests/input/cosmosClaim.in b/Tests/input/cosmosClaim.in new file mode 100644 index 0000000..1443d60 --- /dev/null +++ b/Tests/input/cosmosClaim.in @@ -0,0 +1,3 @@ +Claim +dir +exit diff --git a/Tests/makedisks.sh b/Tests/makedisks.sh index cabb9ab..5f80996 100755 --- a/Tests/makedisks.sh +++ b/Tests/makedisks.sh @@ -194,6 +194,33 @@ awk 'BEGIN { for (i = 0; i < 30; i++) printf "line %02d: ABCDEFGHIJKLMNOPQRSTUVW "$ROOT/Programs/CosmOS/Apps/More.asm" -o "$WORK/More.sbx" >/dev/null "$TOOL" put "$DISKS/type.img" "$WORK/More.sbx" >/dev/null +# A disk for Copy and Compare. The files mark all three shapes a streamed tool has to +# distinguish: no blocks, an exact whole block, and a part block. The large pair says the +# programs do not quietly depend on Data Memory being able to hold either input, and they +# differ at one byte while keeping the same length so Compare has to inspect content rather +# than stopping at the descriptor. +"$TOOL" format "$DISKS/copycompare.img" 1024 4 >/dev/null +"$TOOL" mkdir "$DISKS/copycompare.img" /Apps >/dev/null +"$TOOL" mkdir "$DISKS/copycompare.img" /Input >/dev/null +"$TOOL" mkdir "$DISKS/copycompare.img" /Output >/dev/null +"$ROOT/Assembler" -I "$ROOT/Programs/CosmOS/Source" \ + "$ROOT/Programs/CosmOS/Apps/Copy.asm" -o "$WORK/Copy.sbx" >/dev/null +"$ROOT/Assembler" -I "$ROOT/Programs/CosmOS/Source" \ + "$ROOT/Programs/CosmOS/Apps/Compare.asm" -o "$WORK/Compare.sbx" >/dev/null +"$TOOL" put "$DISKS/copycompare.img" "$WORK/Copy.sbx" /Apps/Copy.sbx >/dev/null +"$TOOL" put "$DISKS/copycompare.img" "$WORK/Compare.sbx" /Apps/Compare.sbx >/dev/null +: > copy-empty.dat +awk 'BEGIN { for (i = 0; i < 256; i++) printf "%c", 65 + (i % 26) }' > copy-exact.dat +awk 'BEGIN { for (i = 0; i < 257; i++) printf "%c", 65 + (i % 26) }' > copy-tail.dat +awk 'BEGIN { for (i = 0; i < 84000; i++) printf "%c", 65 + (i % 26) }' > copy-large.dat +awk 'BEGIN { for (i = 0; i < 84000; i++) printf "%c", (i == 65535 ? 33 : 65 + (i % 26)) }' \ + > copy-different.dat +"$TOOL" put "$DISKS/copycompare.img" copy-empty.dat /Input/empty.dat >/dev/null +"$TOOL" put "$DISKS/copycompare.img" copy-exact.dat /Input/exact.dat >/dev/null +"$TOOL" put "$DISKS/copycompare.img" copy-tail.dat /Input/tail.dat >/dev/null +"$TOOL" put "$DISKS/copycompare.img" copy-large.dat /Input/large.dat >/dev/null +"$TOOL" put "$DISKS/copycompare.img" copy-different.dat /Input/different.dat >/dev/null + # A disk with directories on it, which is the whole of what version two adds. Built by the # host tool, because at this rung the machine can read a tree and not yet make one - and # that split is the point: the two implementations are checked against each other rather @@ -219,6 +246,13 @@ awk 'BEGIN { for (i = 0; i < 30; i++) printf "line %02d: ABCDEFGHIJKLMNOPQRSTUVW printf 'this is not a program' > rooted.txt "$TOOL" put "$DISKS/tree.img" rooted.txt >/dev/null +# A disk for the program that tries to commit a file bigger than it reserved. Its own, +# because what it leaves behind is a file whose size is the thing being checked. +"$TOOL" format "$DISKS/claim.img" 64 2 >/dev/null +"$ROOT/Assembler" -I "$ROOT/Programs/CosmOS/Source" \ + "$ROOT/Programs/CosmOS/Apps/Claim.asm" -o "$WORK/Claim.sbx" >/dev/null +"$TOOL" put "$DISKS/claim.img" "$WORK/Claim.sbx" >/dev/null + # A blank disk for the machine to build a tree on itself. It starts as a version ONE disk # with nothing at all on it, because half of what this checks is that making the first # directory raises the version - the number says what is on a disk rather than what made diff --git a/Tests/manifest b/Tests/manifest index dca3479..ab96300 100644 --- a/Tests/manifest +++ b/Tests/manifest @@ -329,6 +329,10 @@ cosmosType | CosmOS/Source/cosmos.asm | run | cosmosTyp # with Space, and one line with Return. The input is intentionally packed so that the one # byte the pager consumes leaves the next shell command immediately behind it. cosmosMore | CosmOS/Source/cosmos.asm | run | cosmosMore.in | - | disks/type.img +# Copy and Compare exercise both halves of block streaming together. Empty, exact-block, +# part-block and 84,000-byte files are copied through one buffer; Compare checks the copies +# and a same-sized file whose only difference is deep into the input. +cosmosCopyCompare | CosmOS/Source/cosmos.asm | run | cosmosCopyCompare.in | - | disks/copycompare.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. @@ -348,6 +352,23 @@ cosmosTree | CosmOS/Source/cosmos.asm | run | cosmosTre # hold no blocks and must therefore be in nobody's way when a run of free ones is wanted. # The listing afterwards says where it landed and how big it is. cosmosTreeWrite | CosmOS/Source/cosmos.asm | run | cosmosTreeWrite.in | - | disks/treewrite.img +# Committing a streamed file bigger than the room reserved for it, which must be refused. +# +# osFileStart sets an extent aside and osFileWrite already refuses a block index outside it, +# so writing off the end is barred. This is the same neighbour reached the other way: claim +# the extra block at commit rather than write into it. A directory entry is the only record +# of what a file owns, so an entry claiming a block it was never given simply owns it, and +# so does whatever owned it before - both files then look perfectly well formed. +# +# The size is compared and not the room it takes up, because those differ: one block and a +# tail occupies what two whole blocks occupy, so bounding the blocks alone would allow a +# file to report two hundred and forty six bytes that were never written to it. The +# reservation here is one block and a tail of ten, and the claim is two whole blocks - the +# same two blocks, and a bigger lie. +# +# The listing at the end is the point: 266 bytes, which is what was reserved and what was +# written, after the refusal and the honest commit that follows it. +cosmosClaim | CosmOS/Source/cosmos.asm | run | cosmosClaim.in | - | disks/claim.img # The machine building its own tree. It starts with a blank version one disk and makes # every directory on it, which is the half of the filesystem the machine could only read # until now. @@ -422,7 +443,10 @@ app-Stream | CosmOS/Apps/Stream.asm | assemble | - app-Type | CosmOS/Apps/Type.asm | assemble | - | - app-Wander | CosmOS/Apps/Wander.asm | assemble | - | - app-Pour | CosmOS/Apps/Pour.asm | assemble | - | - +app-Claim | CosmOS/Apps/Claim.asm | assemble | - | - app-More | CosmOS/Apps/More.asm | assemble | - | - +app-Copy | CosmOS/Apps/Copy.asm | assemble | - | - +app-Compare | CosmOS/Apps/Compare.asm | assemble | - | - # The assembler that runs on the machine, and its parts. Checked on their own so that a # failure reads as "it does not assemble" rather than as a broken disk image. asm-Asm | CosmOS/Assembler/Asm.asm | assemble | - | -