Daniel Samson
5a5146ec13
SMAP makes the rule the copy layer has followed since it was written into a rule the hardware keeps. A ring-0 read or write of a user page now faults, so any code that reaches for a user pointer directly fails the first time it runs rather than the first time someone attacks it — and the suite becomes the enforcement test, because every case exercises the kernel with the bit on. Nothing had to be fixed to turn it on, which is the retrospective proof that the nine stragglers converted earlier were all of them. The interrupt entry needed one instruction first. Hardware does not clear the alignment-check flag on its way into a handler, and ring 3 sets that flag freely, so a process could have taken an interrupt with SMAP suspended for the duration. The system call path was already covered — its flag mask clears it — but the interrupt path needed a `clac`, which cannot simply be assembled in: it is an invalid instruction on a processor without SMAP, and danos boots on those too. So the entry ships as a three-byte NOP and is patched at boot, through the physmap, because the kernel maps its own text read-only. The ordering that makes that safe is enforced rather than described: the patch sets a flag, and no core will set the SMAP bit until it is true. A translation that fails, or bytes that read back wrong through the address they will actually be fetched from, leave the machine unhardened and saying so — which is the same posture the IOMMU takes, and better than enforcing over an entry path that cannot comply. The patch runs before interrupts are enabled and before any second core exists; a comment says so, because the three bytes pass through an encoding that must never be executed and a future change that moves this later has to deal with that first. Suite 114/114, with a case that reads a user page from ring 0 and requires the fault, and the multi-core case asserting every core that ran work had the bit — the same shape SMEP got, for the same reason: CR4 is per-core, and a hardening is only as wide as its narrowest core.
220 lines
11 KiB
Markdown
220 lines
11 KiB
Markdown
# Coding standards
|
|
|
|
Conventions for danos source. The overriding one, from which most of the rest follows:
|
|
|
|
> **Names are spelled out in full. An identifier is not abbreviated unless the
|
|
> abbreviation is an acronym.**
|
|
|
|
`interruptDispatch`, not `intDisp`. `message_len`, not `msg_len` (`msg` expands, `len`
|
|
is a Zig idiom — see the exceptions). `devices_broker`, not `dev_broker`. `scheduler`, not
|
|
`sched`. The cost of a longer name is paid once, at the keyboard; the cost of a
|
|
cryptic one is paid every time the code is read, by everyone who reads it. In a
|
|
microkernel whose whole argument is that a human can hold each piece in their head,
|
|
that trade is not close.
|
|
|
|
## The rule, precisely
|
|
|
|
**Acronyms and initialisms stay.** They *are* the full name — expanding them would make
|
|
the code worse, not better. `IPC`, `MMIO`, `DMA`, `IRQ`, `TSS`, `GDT`, `IDT`, `APIC`,
|
|
`GSI`, `HPET`, `ACPI`, `PCI`, `EOI`, `BAR`, `ECAM`, `MSI`, `CPU`, `ELF`, `ABI`, `UEFI`,
|
|
`MMU`, `TLB`, `ISR`, `ISA`, `GAS`, `HAL`, `PMM`, `VMM`, `VFS`, `HID`, `HCD`, `SMP`,
|
|
`AML`, `MADT`, `MCFG`, `FADT`, `RSDP`, `XSDT`, `RSDT`, `GOP`, `EDID`, `TSC`, `PIT`,
|
|
`RTC`, `LAPIC`, `SIPI`. In code they carry whatever case the surrounding convention
|
|
demands: `Hal` the type, `hal` the variable, `mapMmio` the function.
|
|
|
|
**Everything else is spelled out.** If it's a word with letters removed, restore them:
|
|
|
|
| Abbreviation | Full |
|
|
|---|---|
|
|
| `proto` | `protocol` |
|
|
| `msg` | `message` |
|
|
| `desc` | `descriptor` |
|
|
| `res` | `resource` |
|
|
| `recv` | `receive` |
|
|
| `buf` | `buffer` |
|
|
| `cur` | `current` |
|
|
| `src` / `dst` | `source` / `destination` |
|
|
| `idx` | `index` |
|
|
| `addr` | `address` |
|
|
| `reg` | `register` |
|
|
| `prev` | `previous` |
|
|
| `cfg` / `config` | `configuration` |
|
|
| `arch` | `architecture` |
|
|
| `sched` | `scheduler` |
|
|
| `dev` | `device` |
|
|
| `sys` / `syscall` | `system` / `system_call` |
|
|
| `info` | `information` |
|
|
| `dt` | `device_tree` |
|
|
| `ep` | `endpoint` |
|
|
| `rt` | `runtime` |
|
|
| `func` | `function` |
|
|
| `phys` / `virt` | `physical` / `virtual` |
|
|
| `wq` | `wait_queue` |
|
|
|
|
This list is illustrative, not exhaustive. The rule is the rule; when you meet a new
|
|
abbreviation, expand it.
|
|
|
|
## Exceptions
|
|
|
|
Three, and only three.
|
|
|
|
1. **Foreign ABI names are spelled exactly as the ABI spells them — but only inside
|
|
the layer that *is* that ABI.** A function that *is* the C or POSIX interface keeps
|
|
its name: `fopen`, `fwrite`, `fread`, `malloc`, `calloc`, `realloc`, `free`,
|
|
`memcpy`, `mmap`, `munmap`, `open`, `read`, `write`, `close`, `lseek`, `stat`,
|
|
`errno`, `O_CREAT`. We don't get to rename `fwrite` to `fileWrite` — it wouldn't be
|
|
`fwrite` any more.
|
|
|
|
**This exception is scoped to a file that *is* a foreign ABI, and nothing else.**
|
|
danos has no such file today: the old `library/posix/` compatibility shim was retired
|
|
once its callers moved to the danos-native `file_system`, since a hand-rolled POSIX
|
|
layer is premature until danos actually needs it (see
|
|
[zig-self-hosting.md](zig-self-hosting.md)). The exception will apply again to the
|
|
`std.os.danos` seam when danos becomes a real Zig target — that module *is* the C-ABI
|
|
`system` interface, so it keeps `open`/`read`/`errno`/`O_CREAT`. **Everywhere else,
|
|
Zig/danos naming applies with no exception**: a concept POSIX also has gets a danos
|
|
name — the VFS wire protocol carries a `FileStatus`, not a `Stat`, and a `create`
|
|
flag, not `O_CREAT`; the boundary is where `stat`→`status` and `O_CREAT`→`create` get
|
|
mapped. (The `syscall` *wrappers* elsewhere are not an exception — they wrap the
|
|
private danos ABI, so they use danos names.)
|
|
|
|
2. **Zig idioms are spelled the way Zig spells them.** Three names are the language's,
|
|
not ours, and are left alone:
|
|
- **`init` / `deinit`** — the constructor convention (`std.ArrayList.init`), not a
|
|
shortening of "initialize".
|
|
- **`len` / `ptr`** — the slice field names (`slice.len`, `slice.ptr`). Our own
|
|
structs use bare `len`/`ptr` fields to mirror them, so a reader carries one
|
|
mental model. (Compounds still expand: a field is `message_len`, not
|
|
`message_length` — `len` is kept, `msg` is not.)
|
|
- The builtins (`@min`, `@max`, `@memcpy`) and `allocator.alloc` / `.create` are
|
|
Zig's spelling.
|
|
|
|
The rule governs the names *we* coin.
|
|
|
|
3. **Single-letter variables in a trivial local scope.** `for (items) |item, i|` may
|
|
keep `i`; a coordinate may be `x`, `y`. The moment the scope is big enough that the
|
|
letter's meaning isn't obvious on sight, give it a real name. When in doubt, name it.
|
|
|
|
That's all — no Unix-abbreviation exception. The source directories are full words
|
|
(`system`, `library`, not `src`/`lib`), and there is no daemon `d` suffix: a driver
|
|
lives in `system/drivers/`, a service in `system/services/`, and a test fixture in
|
|
`test/system/services/` (the repo path *is* its path on the boot volume), so the
|
|
*location* already says what it is. Encoding the role in the name too (`busd`, `fatd`) is
|
|
redundant — the program is just `ps2-bus`, `fat`. Don't put in a name what its directory
|
|
already tells you.
|
|
|
|
## A note on collisions
|
|
|
|
Two identifiers can legitimately expand to the same word. When they do, keep both
|
|
meaningful by renaming one to its *specific* identity rather than the generic
|
|
expansion. Two cases resolved this way:
|
|
|
|
- The `config` module (compile-time tunables — `maximum_cpus`, `timer_hz`) would
|
|
collide with `cfg` (a `PlatformConfiguration` value) at `configuration`. The module
|
|
became **`parameters`**, which is what it holds.
|
|
- The kernel `device.zig` module would collide with `dev` (a device value) at
|
|
`device`. The module alias became **`device_model`**, which is what it is — the
|
|
device data model (`Device`, `DeviceTree`, `ResourceKind`).
|
|
- The `Namespace` module alias (`ns`/`nsp` across the AML files) collides with a
|
|
`Namespace` **instance**. Resolved by dropping the module alias entirely — the two
|
|
types it provided are imported directly (`const Node = @import("namespace.zig").Node;`)
|
|
— which frees `namespace` for the instance.
|
|
|
|
A related case is one abbreviation with two meanings. In the AML code, `op` means
|
|
**opcode** (`opcodes.zig`, the `*_opcode` constants) but `Op` in `BinaryOperation` /
|
|
`LogicOperation` means **operation** — distinguished by case. The per-opcode parser
|
|
handlers, formerly `opName`/`opField`, are `parseName`/`parseField`: they *parse* the
|
|
opcode's structure, which says what they do without overloading "op".
|
|
|
|
## Case and file names
|
|
|
|
Within those spelling rules, follow Zig's own conventions:
|
|
|
|
- **Types** — `PascalCase`: `DeviceDescriptor`, `Endpoint`, `WaitQueue`.
|
|
- **Functions** — `camelCase`: `mapUserDeviceInto`, `notifyFromIsr`.
|
|
- **Variables, fields, constants** — `snake_case`: `message_length`, `devices_broker`,
|
|
`notify_badge_bit`.
|
|
|
|
**File names are `kebab-case`.** A file named for a multi-word thing hyphenates it:
|
|
`device-model.zig`, `ipc-synchronous.zig`, `vfs-protocol.zig`, `devices-broker.zig`. A
|
|
single word or acronym needs no hyphen: `scheduler.zig`, `paging.zig`, `apic.zig`,
|
|
`idt.zig`. (The module *alias* a file is imported under still follows the code
|
|
conventions above — `snake_case` — because it's an identifier, not a filename.)
|
|
|
|
**A sub-project's entry point repeats its directory's name** — `init/init.zig`,
|
|
`pci/pci.zig`, `ps2-bus/ps2-bus.zig` — and the sub-project is addressed by the
|
|
*directory* (`system/services/init`, `library/device/pci`,
|
|
`test/system/services/vfs-test`), with the repeated leaf resolving away. See the
|
|
repository-layout section of [README.md](README.md).
|
|
|
|
## Named values, not magic numbers
|
|
|
|
The naming rule has a twin: **a value with meaning gets a name, too.** The same
|
|
principle drives both — a reader should never have to leave the code to understand it.
|
|
An abbreviated *name* forces a reader to guess; a bare *number* forces them worse, out
|
|
to a spec or a header or a comment three files away, to learn what the value even *is*.
|
|
If `0x0C` is the PCI serial-bus class, the code says `BaseClass.serial_bus`, not `0x0C`;
|
|
if `0x04` is the ACPI IRQ resource descriptor, it says `SmallResourceType.irq`, not
|
|
`0x04`. The number is an implementation detail of the name — recorded once, where the
|
|
name is defined, and never spelled again at a use site.
|
|
|
|
**Prefer an `enum`** when the values form a set (device classes, AML opcodes, resource
|
|
descriptor types, states): the type then also says *which* set a value belongs to, and
|
|
the compiler rejects a value from the wrong one. A lone `pub const` with a descriptive
|
|
name suffices for a one-off (`const large_descriptor_bit = 0x80`). Reach for the enum
|
|
the moment code elsewhere compares against, packs, or produces the value — a packed PCI
|
|
class triple is written from named parts (`.serial_bus`, `.usb`, `.xhci`), never as
|
|
`0x0C_03_30` under a comment that decodes the bytes.
|
|
|
|
The exceptions are the numbers that carry no hidden meaning: `0` and `1` as plain zero
|
|
and one, an index step, a field width, a bit shift. `x + 1`, `buffer[0]`, and `<< 8`
|
|
need no christening — there is nothing to look up. The test is exactly the naming test:
|
|
*would a reader have to look this up to know what it means?* If yes, name it. This is
|
|
what `opcodes.zig`'s `*_opcode` constants, `acpi-ids`'s `HardwareId`, and `pci-class`'s
|
|
class enums already are — reference data defined once and named everywhere it is used.
|
|
|
|
## Why acronyms are the line
|
|
|
|
Because an acronym has no letters to restore. `MMIO` doesn't become "memory mapped
|
|
input output" in code — that expansion is what the acronym *is for*. But `msg` is just
|
|
`message` with three letters stolen, and stealing them buys nothing a reader wants. The
|
|
test for "is this an abbreviation I must expand" is simply: *is there a longer word this
|
|
is a clipped form of?* If yes, write the word. If it's an initialism standing in for a
|
|
phrase, leave it.
|
|
|
|
## Kernel code touches user memory only through `user-memory`
|
|
|
|
A syscall argument is an attacker-controlled integer. Kernel code never
|
|
dereferences one: every read of a process's memory goes through
|
|
`copyFromUser` and every write through `copyToUser`
|
|
(`system/kernel/user-memory.zig`), which walk that address space's page tables
|
|
and move the bytes through the physmap, with the permissions ring 3 itself
|
|
would face. A bad pointer then fails the call instead of faulting the kernel,
|
|
and a struct pulled in once cannot change underneath the checks that follow it.
|
|
|
|
This is not a review convention — CR4.SMAP enforces it in hardware
|
|
(`docs/os-development/smep-smap.md`), so a raw dereference of a user address is
|
|
a #PF with a kernel instruction pointer the first time the QEMU suite reaches
|
|
it. Which is also why **there is no `stac` in this tree, and never should be**:
|
|
`stac` suspends exactly that enforcement, the copy layer needs no such window
|
|
by construction, and a change that adds one has removed the guarantee rather
|
|
than worked around a limitation. The same goes for the boot-time `clac` patch
|
|
at the interrupt entry — it exists so that ring 3 cannot suspend SMAP either,
|
|
by taking an interrupt with `EFLAGS.AC` set.
|
|
|
|
## Zen of Zig
|
|
|
|
* Communicate intent precisely.
|
|
* Edge cases matter.
|
|
* Favor reading code over writing code.
|
|
* Only one obvious way to do things.
|
|
* Runtime crashes are better than bugs.
|
|
* Compile errors are better than runtime crashes.
|
|
* Incremental improvements.
|
|
* Avoid local maximums.
|
|
* Reduce the amount one must remember.
|
|
* Focus on code rather than style.
|
|
* Resource allocation may fail; resource deallocation must succeed.
|
|
* Memory is a resource.
|
|
* Together we serve the users.
|