From 9700a7b404c9181e05604c285345b7949a68f42c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 16:41:00 +0000 Subject: [PATCH] fix: gpt_read checks every entry, and says when there are more than max (#180) gpt_read() stopped at max used entries (16 for the identity code, 4 in the mint): a disk whose StarshipOS partitions came after the first max read as having none, and the entries after max were never range-checked. Now every entry is checked and every used one counted; a good table with more used entries than max is GPT_MANY (the first max in parts), and the identity code says "GPT: a USB drive has more than 16 partitions -- not used". Other callers treat anything but GPT_OK as before. Tested first, seen to fail: qemu_session.py gpt_many -- a data stick with its two partitions as entries 17 and 18 after 16 others, and the same with an entry 19 past the usable range: both "not this system's stick" before (silently cut, entry 19 never checked; logs/20261112-162236). After: "more than 16 partitions" and "a damaged partition table" (logs/20261112-162921). Regressions: gpt_bad_entries, gpt_foreign PASS. gpt_zuse FAILS, on a stale expectation unrelated to this change -- an identity-only GPT drive with no system stick has been held silently as a possible real Zuse since #92 Task 4; its log is kept here. The gpt_many images are made before the boot: a first version made them after, failed in sgdisk and left its QEMU running, stopped by hand (logs/20261112-162003). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017RzZgrf7V7Zk2iczBXDRbm --- .../runs/doe-amd64-20261112-162003.csv | 0 .../runs/doe-amd64-20261112-162236.csv | 0 .../runs/doe-amd64-20261112-162921.csv | 0 .../runs/doe-amd64-20261112-163159.csv | 0 .../runs/doe-amd64-20261112-163442.csv | 0 .../runs/doe-amd64-20261112-163719.csv | 0 kernel/include/starkernel/gpt.h | 8 +++- kernel/src/gpt.c | 12 +++-- kernel/src/v4/sk_v4_identity.c | 4 ++ .../amd64/qemu-amd64-20261112-162003.log | 3 ++ .../amd64/qemu-amd64-20261112-162236.log | 3 ++ .../amd64/qemu-amd64-20261112-162921.log | 3 ++ .../amd64/qemu-amd64-20261112-163159.log | 3 ++ .../amd64/qemu-amd64-20261112-163442.log | 3 ++ .../amd64/qemu-amd64-20261112-163719.log | 3 ++ scripts/qemu_session.py | 48 ++++++++++++++++++- 16 files changed, 83 insertions(+), 7 deletions(-) create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-162003.csv create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-162236.csv create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-162921.csv create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-163159.csv create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-163442.csv create mode 100644 experiments/bare_metal/runs/doe-amd64-20261112-163719.csv create mode 100644 logs/20261112-162003/amd64/qemu-amd64-20261112-162003.log create mode 100644 logs/20261112-162236/amd64/qemu-amd64-20261112-162236.log create mode 100644 logs/20261112-162921/amd64/qemu-amd64-20261112-162921.log create mode 100644 logs/20261112-163159/amd64/qemu-amd64-20261112-163159.log create mode 100644 logs/20261112-163442/amd64/qemu-amd64-20261112-163442.log create mode 100644 logs/20261112-163719/amd64/qemu-amd64-20261112-163719.log diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-162003.csv b/experiments/bare_metal/runs/doe-amd64-20261112-162003.csv new file mode 100644 index 00000000..e69de29b diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-162236.csv b/experiments/bare_metal/runs/doe-amd64-20261112-162236.csv new file mode 100644 index 00000000..e69de29b diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-162921.csv b/experiments/bare_metal/runs/doe-amd64-20261112-162921.csv new file mode 100644 index 00000000..e69de29b diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-163159.csv b/experiments/bare_metal/runs/doe-amd64-20261112-163159.csv new file mode 100644 index 00000000..e69de29b diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-163442.csv b/experiments/bare_metal/runs/doe-amd64-20261112-163442.csv new file mode 100644 index 00000000..e69de29b diff --git a/experiments/bare_metal/runs/doe-amd64-20261112-163719.csv b/experiments/bare_metal/runs/doe-amd64-20261112-163719.csv new file mode 100644 index 00000000..e69de29b diff --git a/kernel/include/starkernel/gpt.h b/kernel/include/starkernel/gpt.h index e3f849ff..0d792011 100644 --- a/kernel/include/starkernel/gpt.h +++ b/kernel/include/starkernel/gpt.h @@ -26,10 +26,14 @@ typedef struct { #define GPT_BAD 2 /* a protective MBR, but its header or entry array does not check out: not used */ #define GPT_EIO 3 /* its first block could not be read: not used; gpt_write_one: nothing was written */ #define GPT_PART 4 /* gpt_write_one only: a write failed after others were made -- the disk is changed (#168) */ +#define GPT_MANY 5 /* gpt_read: a valid GPT with more used entries than max -- every entry checked, none cut (#180) */ /* Read `disk`'s protective MBR, primary GPT header and partition-entry array, - * checking both CRC-32s, the revision, the sizes and the geometry. On GPT_OK, - * `*count` partitions with a non-zero type (at most `max`) are in `parts`. + * checking both CRC-32s, the revision, the sizes and the geometry, and every + * entry's range. On GPT_OK, `*count` partitions with a non-zero type (at most + * `max`) are in `parts`. GPT_MANY when the table is good but has more used + * entries than `max`: the first `max` are in `parts`, `*count` is `max`, and the + * caller says so rather than taking the disk as having only those (#180). * Reads only. */ int gpt_read(blkio_dev_t *disk, gpt_part_t *parts, unsigned max, unsigned *count); diff --git a/kernel/src/gpt.c b/kernel/src/gpt.c index 83eaf4ce..12b47ede 100644 --- a/kernel/src/gpt.c +++ b/kernel/src/gpt.c @@ -124,17 +124,21 @@ int gpt_read(blkio_dev_t *disk, gpt_part_t *parts, unsigned max, unsigned *count } if (gpt_crc32(g_gpt_entries, ebytes) != ecrc) return GPT_BAD; - for (i = 0; i < nent && n < max; i++) { + /* every entry is checked, and every used one counted -- not only the first `max` (#180) */ + for (i = 0; i < nent; i++) { const uint8_t *e = g_gpt_entries + (size_t)i * esize; static const uint8_t zero[16]; uint64_t first = le64(e + 32), last = le64(e + 40); if (memcmp(e, zero, 16) == 0) continue; /* an unused entry */ if (first < first_usable || last > last_usable || first > last) return GPT_BAD; - memcpy(parts[n].type, e, 16); - parts[n].first_lba = first; - parts[n].last_lba = last; + if (n < max) { + memcpy(parts[n].type, e, 16); + parts[n].first_lba = first; + parts[n].last_lba = last; + } n++; } + if (n > max) { *count = max; return GPT_MANY; } /* more than the caller can hold: said, not cut */ *count = n; memcpy(g_gpt_disk_guid, h + 56, 16); g_gpt_alternate_lba = le64(h + 32); diff --git a/kernel/src/v4/sk_v4_identity.c b/kernel/src/v4/sk_v4_identity.c index 63e3553a..cb79c1c7 100644 --- a/kernel/src/v4/sk_v4_identity.c +++ b/kernel/src/v4/sk_v4_identity.c @@ -335,6 +335,10 @@ static int sk_v4_drive_arrived(sk_v4_drive_t *d) g_offered = d; return 1; } + if (gpt_rc == GPT_MANY) { /* SK_V4_GPT_PARTS: 16 (#180) */ + console_println("GPT: a USB drive has more than 16 partitions -- not used"); + return 1; + } if (gpt_rc == GPT_BAD) { console_println("GPT: a USB drive has a damaged partition table -- not used"); return 1; diff --git a/logs/20261112-162003/amd64/qemu-amd64-20261112-162003.log b/logs/20261112-162003/amd64/qemu-amd64-20261112-162003.log new file mode 100644 index 00000000..2344edc7 --- /dev/null +++ b/logs/20261112-162003/amd64/qemu-amd64-20261112-162003.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cefc9f7f4421f15ef68a34debabd072e96066f5c0d9859d80bce167c38b508f8 +size 6315 diff --git a/logs/20261112-162236/amd64/qemu-amd64-20261112-162236.log b/logs/20261112-162236/amd64/qemu-amd64-20261112-162236.log new file mode 100644 index 00000000..10f43808 --- /dev/null +++ b/logs/20261112-162236/amd64/qemu-amd64-20261112-162236.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9283ffd69757a53ab5f25ab27ee04dbfb09548db2e6ab39256569ade9bdf19dc +size 6489 diff --git a/logs/20261112-162921/amd64/qemu-amd64-20261112-162921.log b/logs/20261112-162921/amd64/qemu-amd64-20261112-162921.log new file mode 100644 index 00000000..2935c471 --- /dev/null +++ b/logs/20261112-162921/amd64/qemu-amd64-20261112-162921.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7f493d38accab509f7b84406e21684757b3c74b5d19912485698ca0788cfbee1 +size 6491 diff --git a/logs/20261112-163159/amd64/qemu-amd64-20261112-163159.log b/logs/20261112-163159/amd64/qemu-amd64-20261112-163159.log new file mode 100644 index 00000000..5c0914f5 --- /dev/null +++ b/logs/20261112-163159/amd64/qemu-amd64-20261112-163159.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1d64d3e96067f8237580f6283694c9b20f074c300a2df93dd52a66c8fb0767ec +size 6404 diff --git a/logs/20261112-163442/amd64/qemu-amd64-20261112-163442.log b/logs/20261112-163442/amd64/qemu-amd64-20261112-163442.log new file mode 100644 index 00000000..a4b43e3f --- /dev/null +++ b/logs/20261112-163442/amd64/qemu-amd64-20261112-163442.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b7199aa311daf6f3f707435b557aa3edd2ebd4aa3c61f52c93faaeb24dd660cd +size 6399 diff --git a/logs/20261112-163719/amd64/qemu-amd64-20261112-163719.log b/logs/20261112-163719/amd64/qemu-amd64-20261112-163719.log new file mode 100644 index 00000000..20cbbe3a --- /dev/null +++ b/logs/20261112-163719/amd64/qemu-amd64-20261112-163719.log @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:28f1a934e0a276197655a4ed3f9c313afac6dc2f6a57bce75a3b37511b7d8664 +size 6344 diff --git a/scripts/qemu_session.py b/scripts/qemu_session.py index 5a1edcf2..9a64b476 100755 --- a/scripts/qemu_session.py +++ b/scripts/qemu_session.py @@ -87,6 +87,10 @@ the stick's, plan 2026-10-26-stick-1-screen-and-keyboard.md): no ok> for 30 s; the new one added: the prompt mint_fail the fixed screen; R, a stick whose writes all fail (blkdebug): "failed -- nothing changed", the screen again; the old stick added: the prompt (Review Focus 4) + gpt_many no drive at boot; a data stick whose two StarshipOS partitions are entries 17 and 18, + after 16 others: "GPT: a USB drive has more than 16 partitions -- not used", not cut + to 16 (#180); pulled; the same with an entry 19 outside the usable range: + "a damaged partition table" -- every entry checked (#180) ident_attach_fail no drive at boot; a fresh data stick hot-plugged whose reads at its identity partition's first sector (LBA 2048) fail, so that partition cannot be attached after Artemis is: a message naming the identity partition, Artemis let go -- both or @@ -137,7 +141,7 @@ ap.add_argument("--target", default="qemu") # qemu-stick: boot the PC stick ap.add_argument("--stick-gib", type=int, default=None) # qemu-stick: the copy extended to N GiB a = ap.parse_args() -NO_ZUSE_MODES = {"ident_attach_fail", "gpt_zuse", "gpt_hotplug", "gpt_foreign", "gpt_bad_crc", "gpt_bad_entries", "gpt_no_header", +NO_ZUSE_MODES = {"gpt_many", "ident_attach_fail", "gpt_zuse", "gpt_hotplug", "gpt_foreign", "gpt_bad_crc", "gpt_bad_entries", "gpt_no_header", "gpt_read_error", "headless", "hotplug", "midpull", "waitpull", "typeahead", "emergency", "alttab_after_logout", "alttab_before_pull", "emergency_pull", "screen_dark_until_login", "screen_console_after_login", "screen_dark_after_logout", @@ -201,6 +205,32 @@ if a.mode in ("zuse_stick_first", "fallback", "replace", "mint_fail", "mint_part subprocess.run(["make", "-f", "kernel/Makefile", f"ARCH={a.arch}", "datastick", f"DATASTICK_IMG={dstick}"], cwd=ROOT, check=True, stdout=subprocess.DEVNULL) make_args += ["DATASTICK=1", f"DATASTICK_IMG={dstick}", "DATASTICK_DEP="] +if a.mode == "gpt_many": + # made before the boot, so a failure here leaves no QEMU running + import struct, zlib + many = os.path.join(work, "many.img") + subprocess.run(["make", "-f", "kernel/Makefile", f"ARCH={a.arch}", "datastick", f"DATASTICK_IMG={many}"], + cwd=ROOT, check=True, stdout=subprocess.DEVNULL) + sg = ["sgdisk", "-a", "1", "--transpose=1:17", "--transpose=2:18"] + for k in range(1, 17): # 16 fillers in the free sectors after Artemis (the + lo = 3143680 + (k - 1) * 64 # first usable is 2048: none before the identity) + sg += ["-n", f"{k}:{lo}:{lo + 63}", "-t", f"{k}:8300"] + subprocess.run(sg + [many], check=True, stdout=subprocess.DEVNULL) + bad = os.path.join(work, "many-bad.img") + subprocess.run(["cp", "--sparse=always", many, bad], check=True) + with open(bad, "r+b") as fh: # entry 19 past the last usable LBA, CRCs made good + fh.seek(512); hdr = bytearray(fh.read(92)) + ent_lba, nent, esz = struct.unpack_from("", wait_for(PROMPT, 120)) check("no fault", count(r"EXCEPTION|PANIC|panic") == 0) +elif m == "gpt_many": + # #180: gpt_read() stopped reading entries at its max (16 here) and checked none after them + # the two sticks were made before the boot (see "gpt_many" above) + check("no ok> for 20 s with no drive", not wait_for(ANYPROMPT, 20)) + add_stick(1, many, 2) + check("GPT: a USB drive has more than 16 partitions -- not used", + wait_for(r"GPT: a USB drive has more than 16 partitions -- not used", 120), near=r"GPT|USB|Artemis|Zuse") + qcmd("device_del", id="stickdev1") + time.sleep(3) + qcmd("blockdev-del", **{"node-name": "stick1"}) + d = count(r"GPT: a USB drive has a damaged partition table -- not used") + add_stick(2, bad, 2) + check("an entry past the usable range, after the 16th: a damaged partition table", + wait_for(r"GPT: a USB drive has a damaged partition table -- not used", 120, d), near=r"GPT|USB|Artemis|Zuse") + check("no ok> throughout", count(ANYPROMPT) == 0) + elif m == "ident_attach_fail": # #179: the identity partition (LBA 2048 on a data stick) cannot be read, so it cannot be # attached -- after Artemis has been. Both partitions or neither.