Skip to content

Commit b96ce35

Browse files
committed
build: make 32/64-bit truncation & overflow bug classes build failures
These bugs came from values wider than the target word leaking into the unsigned-long value domain. Turn each compile-time-detectable class into a build failure instead of relying on review: - kasld_addr_t: name the kernel-address value domain (== the target word, per the build-for-target CONTRACT in api.h) and route the result/observation model through it. Arch-independent wide bounds stay unsigned long long, consumed by the unsigned-long-long kasld_addr_in_* helpers. - tests/check-truncation: self-validating guard that cross-compiles each TU for a 32-bit target and fails on long-long -> word narrowing (the MAX_PLAUSIBLE_KERNEL_PHYS class). Forces LC_ALL=C for deterministic diagnostics and proves its own detection on every run, so it can't silently rot into a no-op. Wired into make test (and CI via make check). - _Static_assert on KERNEL_PHYS_MAX: catch an N*GB bound that wraps to 0 on a 32-bit arch -- invisible to #if (preprocessor uses intmax_t) and to warning flags (unsigned overflow is defined), but visible to the compiler in the target's own types. Fires on every arch under make cross. - test-cross: -Werror=shift-count-overflow/overflow, so the shift class is caught on the 32-bit test build. - kasld_add_ovf/kasld_mul_ovf: checked-arithmetic helpers over the compiler builtins; convert the parser's sz->hi extent guard as the reference use.
1 parent 41813d2 commit b96ce35

7 files changed

Lines changed: 236 additions & 7 deletions

File tree

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ $(TEST_INT_BIN): $(TEST_DIR)/test_engine_integration.c $(ENGINE_CORE) $(ENGINE_R
297297
test : $(TEST_BIN) $(TEST_EST_BIN) $(TEST_EV_BIN) $(TEST_ENG_BIN) $(TEST_INT_BIN)
298298
@$(TEST_DIR)/run-all
299299
@$(TEST_DIR)/check-self-edges
300+
@$(TEST_DIR)/check-truncation
300301

301302
.PHONY: test-integration
302303
test-integration : $(TEST_INT_BIN)

src/include/kasld/api.h

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,53 @@
1919
#define GB 0x40000000ul
2020
#define TB 0x10000000000ul
2121

22+
/* =========================================================================
23+
* kasld_addr_t — the kernel-address value domain.
24+
*
25+
* CONTRACT: kasld is built for, and run on, its target architecture (a cross
26+
* compiler may produce the binary, but it executes on the target's kernel).
27+
* So the build word == the target kernel word, and a kernel address is exactly
28+
* `unsigned long`. kasld_addr_t names that domain: use it for stored addresses
29+
* (result/observation lo/hi/sample/base_align, ...) so the intent is explicit
30+
* and the type lives in one place.
31+
*
32+
* Two things deliberately do NOT use kasld_addr_t:
33+
* - values that are arch-INDEPENDENT and may exceed any word (plausibility
34+
* ceilings such as 1<<50, slot math): those are `unsigned long long`, and
35+
* are consumed by `unsigned long long`-parameter helpers (kasld_addr_in_*)
36+
* so a wide bound is never silently truncated to the word; and
37+
* - real machine pointers / syscall operands at the hardware boundary, which
38+
* are the platform's `unsigned long`/`uintptr_t`/`void *` by definition.
39+
* ========================================================================= */
40+
typedef unsigned long kasld_addr_t;
41+
42+
/* Overflow-checked unsigned word arithmetic. Returns 1 if a (+|*) b overflows
43+
* the word — the wrapped result is still written to *out — and 0 otherwise.
44+
* Prefer these to hand-rolling a `b > ULONG_MAX - a` pre-check: they compute
45+
* and check in one step and can't be got subtly wrong. Uses the compiler
46+
* builtin where available; the fallback is the standard wrap test. */
47+
#if defined(__GNUC__) || defined(__clang__)
48+
static inline int kasld_add_ovf(unsigned long a, unsigned long b,
49+
unsigned long *out) {
50+
return __builtin_add_overflow(a, b, out);
51+
}
52+
static inline int kasld_mul_ovf(unsigned long a, unsigned long b,
53+
unsigned long *out) {
54+
return __builtin_mul_overflow(a, b, out);
55+
}
56+
#else
57+
static inline int kasld_add_ovf(unsigned long a, unsigned long b,
58+
unsigned long *out) {
59+
*out = a + b;
60+
return *out < a;
61+
}
62+
static inline int kasld_mul_ovf(unsigned long a, unsigned long b,
63+
unsigned long *out) {
64+
*out = a * b;
65+
return a != 0 && *out / a != b;
66+
}
67+
#endif
68+
2269
#if defined(__x86_64__) || defined(__amd64__)
2370
#include "arch/x86_64.h"
2471
#elif defined(__i386__)
@@ -76,6 +123,16 @@
76123
#if KERNEL_PHYS_MIN > KERNEL_PHYS_MAX
77124
#error "Defined KERNEL_PHYS_MIN is larger than KERNEL_PHYS_MAX"
78125
#endif
126+
/* Catch an N*GB upper bound that overflowed the word on a 32-bit arch (4*GB
127+
* wraps to 0). A `#if` can't see this: the preprocessor evaluates in intmax_t
128+
* (>= 64-bit), so the wrap that only happens in `unsigned long` is invisible to
129+
* it — and to the relational `#if` above. _Static_assert is evaluated by the
130+
* compiler in the target's own types, so it does see the wrap. (Unsigned
131+
* overflow is defined behaviour, so no warning flag catches it either.) */
132+
__extension__ _Static_assert((unsigned long)KERNEL_PHYS_MAX >
133+
(unsigned long)KERNEL_PHYS_MIN,
134+
"KERNEL_PHYS_MAX <= KERNEL_PHYS_MIN -- an N*GB "
135+
"constant overflowed the 32-bit word?");
79136
#endif
80137

81138
/* DIRECTMAP_STATIC and TEXT_TRACKS_DIRECTMAP must be declared by every arch

src/include/kasld/internal.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ struct result {
124124
enum kasld_region region;
125125
char name[NAME_LEN]; /* "" if no specific instance */
126126

127-
unsigned long lo, hi;
128-
unsigned long sample;
129-
unsigned long base_align;
127+
kasld_addr_t lo, hi;
128+
kasld_addr_t sample;
129+
kasld_addr_t base_align;
130130
uint32_t set_mask;
131131

132132
enum kasld_position pos;

src/include/kasld/observation.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ struct observation {
7373
enum kasld_addr_type type;
7474
enum kasld_region region;
7575
char name[NAME_LEN]; /* "" if no specific instance */
76-
unsigned long lo, hi, sample, base_align;
76+
kasld_addr_t lo, hi, sample, base_align;
7777
uint32_t set_mask;
7878
enum kasld_position pos;
7979
/* OBS_SCALAR: */

src/orchestrator.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,9 +730,9 @@ static int capture_result(const char *line, const char *method,
730730
/* sz requires lo — check before doing arithmetic on p.lo. */
731731
if (!p.seen_lo)
732732
return 0;
733-
if (p.sz == 0 || p.sz - 1 > ULONG_MAX - p.lo)
733+
/* hi = lo + sz - 1, rejecting an empty or wrapping extent in one step. */
734+
if (p.sz == 0 || kasld_add_ovf(p.lo, p.sz - 1, &p.hi))
734735
return 0;
735-
p.hi = p.lo + p.sz - 1;
736736
p.seen_hi = 1;
737737
}
738738

tests/check-truncation

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
#!/bin/sh
2+
# This file is part of KASLD - https://github.com/bcoles/kasld
3+
#
4+
# check-truncation — guard against silent 64-bit -> word narrowing on 32-bit.
5+
#
6+
# kasld's value domain is the target kernel word (kasld_addr_t == unsigned
7+
# long; see the CONTRACT in include/kasld/api.h). A value that is WIDER than
8+
# the word — an `unsigned long long`: an arch-independent bound such as 1<<50,
9+
# or a uint64_t — assigned to or passed as that word-sized domain is silently
10+
# truncated on 32-bit builds. That is the class that produced the
11+
# MAX_PLAUSIBLE_KERNEL_PHYS (1<<50) bug: correct on 64-bit, zero on 32-bit.
12+
#
13+
# The compiler flags this under -Wconversion/-Woverflow, but those are far too
14+
# noisy to enable globally. So this guard compiles each translation unit for a
15+
# 32-bit target and fails on the NARROWING SUBSET only: `long long` -> `long`.
16+
# Keep wide values in the unsigned-long-long bound layer (kasld_addr_in_*),
17+
# which never truncates.
18+
#
19+
# A file that #errors on the 32-bit target (arch-incompatible, e.g. an
20+
# x86_64-only exploit) is skipped: it never builds for 32-bit, so a
21+
# recovery-mode warning is moot. A file that DOES build 32-bit and carries an
22+
# intentional narrowing (a deliberate 64 -> 2x32 pointer split) is listed in
23+
# ALLOW. Skips cleanly when no 32-bit cross toolchain is present.
24+
#
25+
# Usage: tests/check-truncation (also run by `make test`)
26+
# Env: JOBS (default: nproc)
27+
# ---
28+
# <bcoles@gmail.com>
29+
30+
set -u
31+
ROOT=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
32+
33+
# Force ASCII diagnostics. GCC quotes type names with Unicode ‘ ’ under a
34+
# UTF-8 locale (e.g. CI's glibc toolchain) and ASCII ' ' under C — and the
35+
# grep below keys on the quoted type names, so without this the guard silently
36+
# passes wherever the locale is UTF-8. C locale makes the output deterministic.
37+
export LC_ALL=C
38+
39+
# Reviewed, intentional narrowings in genuinely-32-bit translation units.
40+
# Keep alphabetical.
41+
ALLOW="proc_pid_syscall" # deliberate 64-bit register split into two 32-bit halves
42+
43+
# Pick ONE 32-bit (ILP32, sizeof(long)==4) GCC. The narrowing this guard
44+
# catches is sizeof(long)-dependent and therefore identical across every 32-bit
45+
# arch, so a single representative compiler is sufficient — no need to sweep all
46+
# arches (the value-dependent overflow/shift classes are swept on every arch by
47+
# `make cross` / `tests/test-cross` instead). The list MUST stay 32-bit-only: a
48+
# 64-bit compiler would never see the narrowing, silently making the guard a
49+
# no-op. i686 is canonical (CI installs gcc-i686-linux-gnu); the rest are
50+
# fallbacks so any of our cross toolchains satisfies it. (clang is not listed:
51+
# its -Wconversion diagnostic spelling differs; the guard is GCC-based and skips
52+
# where no 32-bit GCC is found.)
53+
CC=
54+
for c in i686-unknown-linux-musl-gcc i686-linux-gnu-gcc i686-linux-musl-gcc \
55+
arm-unknown-linux-musleabi-gcc armv7-unknown-linux-musleabi-gcc \
56+
armeb-linux-musleabi-gcc arm-linux-gnueabihf-gcc arm-linux-gnueabi-gcc \
57+
mipsel-unknown-linux-musl-gcc mips-unknown-linux-musl-gcc \
58+
powerpc-linux-musl-gcc riscv32-linux-musl-gcc; do
59+
command -v "$c" >/dev/null 2>&1 && {
60+
CC=$c
61+
break
62+
}
63+
done
64+
65+
if [ -t 2 ]; then
66+
RED=$(printf '\033[31m')
67+
GREEN=$(printf '\033[32m')
68+
RESET=$(printf '\033[0m')
69+
else
70+
RED=
71+
GREEN=
72+
RESET=
73+
fi
74+
75+
if [ -z "$CC" ]; then
76+
printf '%struncation guard: SKIP%s (no 32-bit cross toolchain)\n' "$GREEN" "$RESET"
77+
exit 0
78+
fi
79+
80+
CFLAGS="-std=c99 -O2 -Wall -Wextra -Wconversion -I$ROOT/src -fsyntax-only"
81+
JOBS=${JOBS:-$(nproc 2>/dev/null || echo 1)}
82+
83+
# The detection, in ONE place (used by both the scan and the self-check below):
84+
# a 64-bit UNSIGNED value narrowed to the unsigned word, in the compiler's
85+
# C-locale (ASCII-quoted) diagnostics. Reads compiler output on stdin, emits the
86+
# matching lines. The value domain is unsigned, so signed narrowings
87+
# (time_t/off_t differences — small relative values) are deliberately excluded.
88+
narrowings() {
89+
grep -E '\[-W(conversion|overflow)\]' |
90+
grep 'long long unsigned int' | grep "to 'long unsigned int'"
91+
}
92+
93+
# Self-check: this guard greps compiler diagnostics, which are sensitive to
94+
# locale (quote style), compiler, and version — a mismatch makes it pass
95+
# SILENTLY (as it did when GCC's UTF-8 Unicode quotes slipped the grep). So
96+
# before vouching, prove detection still fires on a KNOWN narrowing. This also
97+
# rejects an accidentally-64-bit compiler, on which 1<<50 fits the word and
98+
# would not narrow at all.
99+
if ! printf 'unsigned long f(void){unsigned long n=1ull<<50;return n;}\n' |
100+
$CC $CFLAGS -x c - 2>&1 | narrowings >/dev/null; then
101+
printf '%struncation guard: SELF-CHECK FAILED%s — %s did not flag a known\n' \
102+
"$RED" "$RESET" "$CC" >&2
103+
printf '64-bit -> word narrowing, so detection is broken (a diagnostic-format\n' >&2
104+
printf 'or locale change, or a non-32-bit compiler). Refusing to report OK\n' >&2
105+
printf 'rather than pass silently. See narrowings() / LC_ALL in this script.\n' >&2
106+
exit 1
107+
fi
108+
109+
RESDIR=$(mktemp -d)
110+
111+
# Compile one TU; emit "<basename>\n<narrowing lines>" to its .res iff it
112+
# (a) builds for 32-bit (no #error) and (b) has a long-long -> word narrowing
113+
# that is not allowlisted.
114+
check_one() {
115+
f=$1
116+
res=$2
117+
out=$($CC $CFLAGS "$f" 2>&1)
118+
printf '%s\n' "$out" | grep -q "error:" && return # arch-incompatible on 32-bit
119+
hits=$(printf '%s\n' "$out" | narrowings)
120+
[ -n "$hits" ] || return
121+
b=$(basename "$f" .c)
122+
for a in $ALLOW; do [ "$a" = "$b" ] && return; done
123+
{
124+
printf ' %s:\n' "$b"
125+
printf '%s\n' "$hits" | sed 's/^/ /'
126+
} >"$res"
127+
}
128+
129+
n=0
130+
running=0
131+
for f in "$ROOT"/src/*.c "$ROOT"/src/rules/*.c "$ROOT"/src/components/*.c \
132+
"$ROOT"/src/render/*.c; do
133+
[ -f "$f" ] || continue
134+
n=$((n + 1))
135+
check_one "$f" "$RESDIR/$n.res" &
136+
running=$((running + 1))
137+
if [ "$running" -ge "$JOBS" ]; then
138+
wait
139+
running=0
140+
fi
141+
done
142+
wait
143+
144+
fail=0
145+
i=1
146+
while [ "$i" -le "$n" ]; do
147+
if [ -s "$RESDIR/$i.res" ]; then
148+
if [ "$fail" = 0 ]; then
149+
printf '%struncation guard: 64-bit -> word narrowing on 32-bit (%s):%s\n' \
150+
"$RED" "$CC" "$RESET" >&2
151+
fi
152+
cat "$RESDIR/$i.res" >&2
153+
fail=1
154+
fi
155+
i=$((i + 1))
156+
done
157+
rm -rf "$RESDIR"
158+
159+
if [ "$fail" = 1 ]; then
160+
printf 'A value wider than the target word is truncated on 32-bit. Keep it in the\n' >&2
161+
printf 'unsigned-long-long bound layer (kasld_addr_in_*); or, if the narrowing is\n' >&2
162+
printf 'intentional in a 32-bit-only TU, add it to ALLOW in tests/check-truncation.\n' >&2
163+
exit 1
164+
fi
165+
166+
printf '%struncation guard: OK%s (compiled for 32-bit via %s; no silent narrowing)\n' \
167+
"$GREEN" "$RESET" "$CC"

tests/test-cross

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ set -u
2727
ROOT=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
2828
QEMU_DIR=${QEMU_DIR:-/home/user/qemu/build}
2929
OUT=$ROOT/build/tests
30-
CFLAGS=${CFLAGS:--std=c99 -O2 -static -I$ROOT/src}
30+
# -Werror on the overflow/shift classes: these only manifest on a given word
31+
# size, so the cross (esp. 32-bit) compile is where a `1ul << 40` shift or a
32+
# constant overflow gets caught. Kept targeted (not full -Werror) so the test
33+
# TUs' pre-existing unused-static warnings don't turn fatal.
34+
CFLAGS=${CFLAGS:--std=c99 -O2 -static -I$ROOT/src -Werror=shift-count-overflow -Werror=overflow}
3135
# Targets are independent (separate build dir + binaries each), so build/run
3236
# them JOBS at a time. Default to one per core; JOBS=1 restores serial order.
3337
JOBS=${JOBS:-$(nproc 2>/dev/null || echo 1)}

0 commit comments

Comments
 (0)