Skip to content

Commit b384435

Browse files
committed
orchestrator: update code comments
Two comments justified themselves with lists of architectures, and both had drifted. One named four !KASLR_SUPPORTED arches: one of them had been an stubs landed later. The other said the head gap is zero everywhere but arm64/loongarch64, which stopped being true when mips32/mips64 declared STEXT_OFFSET. Neither is a soundness bug — the macros that actually decide, KASLR_DISABLED_PINS_VIRT_TEXT/PHYS and STEXT_OFFSET, are correct on every arch — so both comments now rest on those macros, which a reader can check and which cannot go stale. Also: region_info[] reached the 30 its comment called "under 30", and ten comments spoke in the first or second person.
1 parent 21e9cbe commit b384435

1 file changed

Lines changed: 35 additions & 30 deletions

File tree

src/orchestrator.c

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,8 @@ const struct kasld_cap_leak kasld_cap_leaks[KASLD_N_CAP_LEAKS] = {
839839
{39, "CAP_BPF", "BPF verifier-log pointer"},
840840
};
841841

842-
/* Detect whether we run inside a container and, if so, the runtime. All reads
842+
/* Detect whether the run is inside a container and, if so, the runtime. All
843+
* reads
843844
* are unprivileged and SYSROOT-redirectable. Returns a runtime name or NULL
844845
* (not containerized / undetectable). Marker files are most reliable; cgroup
845846
* path patterns catch the rest. */
@@ -1039,16 +1040,17 @@ static void print_banner(void) {
10391040
VERSION);
10401041
}
10411042

1042-
/* Print the container / confinement lines: whether we are containerized, and
1043-
* the seccomp / capability / no-new-privs state that decides which oracles are
1043+
/* Print the container / confinement lines: whether the vantage is
1044+
* containerized, and the seccomp / capability / no-new-privs state that decides
1045+
* which oracles are
10441046
* reachable here. Descriptive — the offensive-recon complement to the sysctl
10451047
* block above.
10461048
*
10471049
* The detail lines are printed ONLY when the process is actually confined
10481050
* (containerized, a seccomp filter, or no_new_privs). On a bare unprivileged
10491051
* host their values (Seccomp: none, caps: none, no_new_privs: no) are the
1050-
* DEFAULTS, not restrictions — printing them there reads as "you are confined"
1051-
* when you are not, so we suppress them and show just the container status. */
1052+
* DEFAULTS, not restrictions — printing them there reads as confinement where
1053+
* there is none, so they are suppressed and only the container status shows. */
10521054
/* List the cap-gated leaks the effective cap set unlocks (one line each), or
10531055
* nothing if none apply. Shown regardless of confinement: a held cap is a real
10541056
* reachability fact whether or not the process is otherwise restricted. */
@@ -1243,8 +1245,8 @@ int num_results;
12431245
struct scalar_fact_record scalar_facts[MAX_SCALAR_FACTS];
12441246
int num_scalar_facts;
12451247

1246-
/* Look up region enum by wire name. Linear scan over region_info[] — under
1247-
* 30 entries, negligible cost. Returns REGION_UNKNOWN on miss. */
1248+
/* Look up region enum by wire name. Linear scan over region_info[] — a table
1249+
* of a few dozen, negligible cost. Returns REGION_UNKNOWN on miss. */
12481250
static enum kasld_region region_from_wire(const char *s) {
12491251
for (int i = 1; i < REGION__COUNT; i++) {
12501252
if (region_info[i].wire_name && strcmp(region_info[i].wire_name, s) == 0)
@@ -1769,8 +1771,8 @@ static char *extract_elf_section(const char *path, const char *section_name) {
17691771

17701772
int is64 = (e_ident[EI_CLASS] == ELFCLASS64);
17711773

1772-
/* Read ELF header fields we need: e_shoff, e_shentsize, e_shnum,
1773-
* e_shstrndx. Seek past e_ident which we already consumed. */
1774+
/* Read the ELF header fields used below: e_shoff, e_shentsize, e_shnum,
1775+
* e_shstrndx. Seek past e_ident, already consumed. */
17741776
uint64_t e_shoff;
17751777
uint16_t e_shentsize, e_shnum, e_shstrndx;
17761778

@@ -2334,7 +2336,7 @@ static int run_component(const struct component *c) {
23342336
}
23352337

23362338
if (pid == 0) {
2337-
/* Child: new process group so we can kill any grandchildren */
2339+
/* Child: new process group, so any grandchildren die with it */
23382340
setpgid(0, 0);
23392341

23402342
/* Redirect stdout to pipe, merge stderr into stdout. If either dup2
@@ -3135,8 +3137,8 @@ static void fill_mem_likely(const struct estimate *l, unsigned long g_lo,
31353137

31363138
/* The reported text base is the IMAGE BASE (_text). A KERNEL_IMAGE anchor is
31373139
* the image base directly; a KERNEL_TEXT anchor is _stext, normalized down by
3138-
* the head gap (no-op where the gap is 0, i.e. every arch but
3139-
* arm64/loongarch64). Returns 0 when no kernel-image/text base anchor exists.
3140+
* the head gap the arch declares in STEXT_OFFSET (a no-op where that is 0).
3141+
* Returns 0 when no kernel-image/text base anchor exists.
31403142
*/
31413143
static unsigned long anchor_image_base(enum kasld_addr_type type) {
31423144
const struct result *r = select_anchor(type, REGION_KERNEL_IMAGE);
@@ -3606,22 +3608,25 @@ void compute_component_stats(struct summary *s) {
36063608
* -------------------------------------------------------------------------
36073609
*/
36083610
void inject_kaslr_defaults(struct summary *s) {
3609-
/* "Unsupported" is a compile-time property of the arch (KASLR_SUPPORTED=0 on
3610-
* arm32 / ppc64 / riscv32 / sparc); no runtime signal needed. Surface it for
3611-
* the renderer banner, and seed the informational default address from the
3612-
* statically-initialised layout (= KERNEL_VIRT_TEXT_DEFAULT). */
3611+
/* "Unsupported" is a compile-time property of the arch; no runtime signal
3612+
* needed. Surface it for the renderer banner, and seed the informational
3613+
* default address from the statically-initialised layout
3614+
* (= KERNEL_VIRT_TEXT_DEFAULT). */
36133615
s->kaslr.unsupported = !KASLR_SUPPORTED;
36143616
s->kaslr.default_addr = layout.virt_image_base_default;
36153617

36163618
#if !KASLR_SUPPORTED
36173619
/* Surface the compile-time arch-off as SF_VIRT_KASLR_DISABLED +
36183620
* SF_PHYS_KASLR_DISABLED so the engine sees it like any runtime detector
3619-
* signal. Inert today on the four !KASLR_SUPPORTED arches (none satisfies
3620-
* KASLR_DISABLED_PINS_VIRT_TEXT/PHYS — all four are relocating, bootloader
3621-
* can still place the image), so no unsound text-base pin: the renderer's
3622-
* "KASLR not supported" banner + default-addr line shows, the engine
3623-
* refuses to pin. A future !KASLR_SUPPORTED arch that does satisfy one of
3624-
* those macros would pin correctly via the same rule path. */
3621+
* signal.
3622+
*
3623+
* Emitting the facts is not pinning. Whether a KASLR-off fact fixes the text
3624+
* base is KASLR_DISABLED_PINS_VIRT_TEXT / KASLR_DISABLED_PINS_PHYS, which
3625+
* default to 0 and are raised per arch with the rationale beside them; api.h
3626+
* states the contract and what obliges an arch to stay at the default. An
3627+
* arch at the default gets the renderer's "KASLR not supported" banner and
3628+
* default-addr line while the engine refuses to pin; an arch that raises one
3629+
* pins through the same rule path a runtime detector uses. */
36253630
if (num_scalar_facts + 1 < MAX_SCALAR_FACTS) {
36263631
struct scalar_fact_record *fv = &scalar_facts[num_scalar_facts++];
36273632
fv->fact = SF_VIRT_KASLR_DISABLED;
@@ -4358,7 +4363,7 @@ static void engine_sync_authoritative(const struct engine *e) {
43584363
* from the compile-time floor (x86_64 RANDOMIZE_MEMORY). Anchor the rendered
43594364
* memory map's direct-map band at the engine's best-known base (pinned, or
43604365
* the proven lower bound). Gated on lo having actually been raised above the
4361-
* compile-time default, so we never claim more than the engine proved.
4366+
* compile-time default, so the claim never exceeds what the engine proved.
43624367
*
43634368
* Only layout.virt_page_offset moves — NOT layout.virt_kernel_vas_start. On a
43644369
* decoupled arch the direct-map base is the lowest kernel *mapping* but NOT
@@ -4412,8 +4417,8 @@ static void engine_sync_authoritative(const struct engine *e) {
44124417
*
44134418
* 2 GiB is the MODULES_LEN on both arches; absent a per-arch macro, use
44144419
* the literal constant with this rationale. Gated on virt_image_base_max
4415-
* being a meaningful (narrowed-or-pinned) value — we keep the static
4416-
* band when the engine has not narrowed text. */
4420+
* being a meaningful (narrowed-or-pinned) value — the static band stays
4421+
* when the engine has not narrowed text. */
44174422
#define KASLD_MODULES_LEN (2ul * 1024 * 1024 * 1024)
44184423
if (vt->hi > vt->lo || vt->lo > (unsigned long)KASLR_VIRT_TEXT_MIN) {
44194424
#if MODULES_BELOW_TEXT_START
@@ -4509,7 +4514,7 @@ static void engine_sync_authoritative(const struct engine *e) {
45094514
* The compile-time MODULES_START/END is the validation UNION across all
45104515
* in-scope kernel-version layouts -- wide on purpose so no real module
45114516
* leak is silently rejected. When proc_modules or sysfs_module_sections
4512-
* have given us actual module addresses (emitted as VIRT REGION_MODULE
4517+
* have supplied actual module addresses (emitted as VIRT REGION_MODULE
45134518
* or REGION_MODULE_BAND observations), the runtime band lives in a
45144519
* much smaller span. Tightening the rendered/JSON layout to that
45154520
* observed span makes the diagram reflect reality on this kernel, and
@@ -4518,9 +4523,9 @@ static void engine_sync_authoritative(const struct engine *e) {
45184523
* within a ~128 TiB static union).
45194524
*
45204525
* Soundness: the observed bounds are clamped to the validation union;
4521-
* we never widen past what MODULES_START/END allows. If observations
4522-
* fall entirely outside the union (would indicate a kernel layout we
4523-
* don't yet know about), keep the static window — surfacing the
4526+
* the bounds never widen past what MODULES_START/END allows. If
4527+
* observations fall entirely outside the union (which would indicate an
4528+
* unmodelled kernel layout), keep the static window — surfacing the
45244529
* discrepancy via the wider rendering is more useful than silently
45254530
* shrinking to a single bogus point. */
45264531
{
@@ -5029,7 +5034,7 @@ int main(int argc, char *argv[]) {
50295034

50305035
/* Conflict check: at most one of the OPT_SECT_FORMAT flags may be
50315036
* effective. Each format setter already clears its siblings, so this is
5032-
* a courtesy diagnostic — surfacing "you asked for two formats" before
5037+
* a courtesy diagnostic — surfacing that two formats were asked for before
50335038
* silently going with whichever came last. */
50345039
if (json_output + oneline_output + markdown_output > 1) {
50355040
/* Unreachable today because setters clear siblings, but kept as a

0 commit comments

Comments
 (0)