Page MenuHomeFreeBSD

Add sysconf(8) and libbsdconf(3)
Needs ReviewPublic

Authored by dteske on Mon, Jul 6, 11:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Aug 3, 7:16 AM
Unknown Object (File)
Sun, Aug 2, 8:54 PM
Unknown Object (File)
Sun, Aug 2, 7:30 AM
Unknown Object (File)
Sun, Aug 2, 6:33 AM
Unknown Object (File)
Sun, Aug 2, 4:33 AM
Unknown Object (File)
Sat, Aug 1, 5:58 PM
Unknown Object (File)
Sat, Aug 1, 8:44 AM
Unknown Object (File)
Sat, Aug 1, 8:44 AM

Details

Reviewers
kfv_kfv.io
adrian
des
bdrewery
emaste
Group Reviewers
manpages
Summary

Complete the native configuration trinity: sysctl(8) for live kernel
state, sysrc(8) for rc.conf(5), and sysconf(8) for the remaining base
configuration -- loader.conf(5), sysctl.conf(5), and the make.conf(5)
family -- atop libbsdconf(3).

libbsdconf resurrects figpar as a unified reader/writer. Callbacks own
semantics; statements may span multiple lines via backslash continuation;
non-seekable input is spooled; writes are atomic (mkstemp, fsync, rename)
with mode/owner preservation. Format descriptors name each target, its
files, and quoting rules without private parsers. Multi-file targets
follow boot sourcing order; loader chases loader_conf_files as the boot
loader does.

sysconf(8) is the operator-facing tool: name / name=value on a required
target, sysrc-style list edits, make append and list-strike where they
belong, jail/altroot, and a capsicum sandbox for read-only use.

Sysctl writes validate against the running kernel first -- unknown and
read-only OIDs, CTLFLAG_TUN (pointing at the loader target), and CTLTYPE
range checks -- so a typo or overflow does not land in sysctl.conf.

Make and src treat WITH_/WITHOUT_ as presence knobs (as bsd.mkopt.mk /
src.conf(5) do) and warn on the WITH_*=no form that does not disable the
option, so a bad assignment is caught before an /usr/src build surfaces
it.

The rc target passes through to sysrc(8).

Defaults querying (-d/-D/-A) mirrors sysrc for dumps and descriptions on
targets that have a defaults file; named reads already see defaults, and
-A only widens dump scope.

Manuals are split pkg(8)-style (bsdconf/put/format; sysconf plus
per-target pages). ATF coverage exercises the frontend.

Co-authored-by: Faraz Vahedi <kfv@kfv.io>

Test Plan

Standalone builds of the full library + sysconf(8) with
cc -Wall -Wextra -Wshadow -Werror, plus AddressSanitizer/
UndefinedBehaviorSanitizer builds; all functional tests below were run
under the sanitized binary (leak-free).

Every library translation unit also compiles under strict POSIX
(-std=c99 -D_POSIX_C_SOURCE=200809L, FreeBSD undefined) to keep the
Linux portability promise honest.

Functional testing in a fake root (-R; no root privilege required)
with fixtures for the loader, sysctl, make, src, and src-env targets:

  • Every flag exercised: -a -c -e -E -F -f -i -j/-R exclusivity, -k, -l, -L, -n, -N, -q, -v, -x, --help, and `-f -' (stdin).
  • Option placement on both sides of the target keyword.
  • Quoting round-trips against each consumer's actual rules: loader.conf always-quoted with strict equals; sysctl.conf quoted only when required; make.conf family verbatim with += ?= := != operators and empty values (WITHOUT_* idiom).
  • Multi-file semantics: authoritative (last-file) reads, writes to the authoritative file, appends to the default file, removals from every file listing the directive (no unmasking).
  • Backslash-newline continuation: read, and collapse-on-rewrite, without disturbing neighboring statements.
  • Streams: /dev/stdin and fifo reads (spooled, including inside the capsicum sandbox); writes to a fifo or stdin cleanly rejected.
  • Security: .bak symlink-attack refused (O_NOFOLLOW), create-if-missing is O_EXCL-atomic, temp file mode/ownership propagation verified.
  • Live-system sysctl validation: read-only OIDs and loader-only tunables (CTLFLAG_TUN) rejected with a pointer to the loader target while remaining valid assignments still apply.

Both manual pages pass mandoc -Tlint; the three EXAMPLES in bsdconf.3
were extracted, compiled with -Werror under the sanitizers, and run.

Not yet done: a full make buildworld against the new Makefiles
(pending; the library and utility build standalone with the same
sources and flags).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 75357
Build 72240: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Honor loader_conf_files discovery from /boot/defaults/loader.conf (kevans); add sysrc-style -d/-D/-A defaults querying, LOADER_DEFAULTS, and a FILES section

Let -d compose with the dump flags as in sysrc(8): -ad/-Ad/-aDd dump descriptions of every directive in scope; sysctl -ad pairs configured directives with kernel OID descriptions

Allow -A with -D as in sysrc(8): -D's scoping wins, -A keeps its dump-implying role, so -ADd describes all defaults and only defaults

Add rc pass-through to sysrc(8) (all arguments verbatim, unvalidated), sysrc-style name+=word/name-=word list editing for non-make targets, and --version

Defaults-aware reads: the defaults file is always scanned first (like the boot loader), so 'sysconf loader acpi_video_load' reports the effective boot value; the defaults slot is read-only and never appears in -l/-L, write targeting, or removals

Touch-ups: --version prints '1.0 Jul-6,2026' (sysrc format), -h/--help advertise name[[+|-]=value], SYNOPSIS renders name[[+|-]=value] via sysrc.8's mdoc idiom

@kfv_kfv.io are you okay with using the abridged license form shown in style.9 and the freebsd license guide?

Adopt the abridged license form per style(9) and the FreeBSD license guide across all new files (requested by ziaee): copyright lines first, then SPDX-License-Identifier: BSD-2-Clause, dropping the full 2-clause text

Adversarial parser fuzzing (ASan/UBSan) found and fixed two figpar-inherited memory-safety bugs: NULL directive buffer on an empty-name line, and a bsdconf_strexpand() heap over-read/over-write when a value shrinks as backslash escapes collapse (plus a one-byte over-read on a trailing backslash)

lib/libbsdconf/Makefile
16

Listed in the page's .Nm block (bsdconf.3:27).

lib/libbsdconf/bsdconf_string.c
81–82

We're not using it anywhere to distinguish success from failure.

208–210

UB per C23 §7.4.1:

The header <ctype.h> declares several functions useful for classifying and mapping characters.
In all cases the argument is an int, the value of which shall be representable as an unsigned char
or shall equal the value of the macro EOF. If the argument has any other value, the behavior is
undefined.

216

I'm pretty sure this is the same class as the two bugs fixed in Diff 181466.

When the byte after x is not a hex digit, the "unknown escape" branch consumes it unconditionally. If that byte is the NUL terminator, the chr++ at line 247 steps past it and the while (*chr != '\0') loop keeps reading out of bounds until it happens to find a zero byte, so it is not a bounded 1-byte over-read: adjacent heap contents can be copied into the parsed value.

So we can perhaps guard the consumption on *(chr + 1) != '\0' or emit x verbatim for trailing \x with no digits. Your call.

222–229

Similar to the above-mentioned case, UB per C23 §7.4.1.

238

Similar to above-mentioned occurrences, UB per C23 §7.4.1.

273

Same story, UB per C23 §7.4.1.

The return cast on the other hand is just to say the narrowing is deliberate, so it'd keep the line quiet under conversion-class diagnostics; which we don't have enabled at the time, but I still think it'd be a good practice.

I haven't reviewed the rest yet, but I'll go through them shortly and submit my reviews as well. For bsdconf.c and sysconf.c, at least, I've noticed a few things I'd like to share my notes with you. I'll get back to the rest soon again.

In the meantime, .Lb libbsdconf results in unknown library name, so please take care of it contrib/mandoc/lib.in.

This revision now requires changes to proceed.Mon, Jul 27, 7:08 AM
lib/libbsdconf/bsdconf_string.c
207

This is incredibly wasteful. The ctype(3) functions are locale-aware and therefore slow, and the work they are doing here is needlessly duplicated by the later strtoul(3) call. It would be more efficient (and far more readable) to use direct comparisons and convert as you go. It would also save you from having to copy the digits before converting. Same for the octal conversion below.

lib/libbsdconf/bsdconf_string.c
56

Why only allocated memory? Surely a local or global non-const array will work fine? Also, we usually use the term “buffer”, not “block of memory”.

64

Do you think that maybe in 2026 you could do better than crash? Like take a size argument?

132

This function actually unexpands rather than expands. Otherwise it would not be able to operate in-place.

136–137

Passing a constant to this function invokes undefined behavior. Whether or not it segfaults is beside the point. Either way, the compiler won't let you do it, unless you force its hand.

139–143

“may safely share the buffer” is academic since you don't offer the caller a choice. It would be germane if the function took two arguments and the caller could choose to pass the same buffer to both, cf. strunvis(3).

147

That's not how C-style octal escapes work, DEL (ASCII 127) is encoded as \177, not \0177.

161–166

buf, src, dst` instead of source, chr, and pos would be easier to follow.

Please keep in mind that style(9) mandates sorting variables first in order of decreasing size, then alphabetically.

261

Why only allocated memory? Surely a local or global non-const array will work fine?

Same as above: undefined behaviour per C23 §7.4.1.

lib/libbsdconf/bsdconf.c
212–213
236
273
305
313
463
475
483
dteske marked 24 inline comments as done.

kfv + des: ctype UB casts; strunexpand(dst, src) with convert-as-you-go
hex/octal; size-aware replaceall; mandoc .Lb + format_register MLINK.

lib/libbsdconf/bsdconf.c
208

We are not checking read(2) for failure, so bsdconf_fparse() could spin forever when it fails. read() returns −1 on error, which != 0 holds true for, so every loop treats a failure as "more data available". For example, assume a descriptor open on a directory (lseek(2) succeeds, so the spool path is not taken; read(2) then fails with EISDIR forever).

Worse, the directive-length loop below increments n on every failed read and the buffer is grown to n + 1, so it is an unbounded realloc() growth loop as well as a hang. Besides, since n is uint32_t, at 4 GiB it wraps and read(fd, directive, n) is then called with a length far exceeding the allocation. bsdconf_spool() and bsdconf_readfile() both handle r < 0 correctly with if (r < 0) { if (errno == EINTR) continue; goto fail; }.

kfv: honor read(2) failures in bsdconf_fparse (EINTR retry / fail via read1/readn); no hang or unbounded realloc on EISDIR

lib/libbsdconf/bsdconf.c
540–541

It's nothing critical, but I recommend adding the t > value guard here for parity with the upcoming occurrences for #/; trim. Safe today as the value scan always leaves t on real content first, but I think it could be a latent one-byte underflow if that invariant ever broke.

lib/libbsdconf/bsdconf_put.c
192

After counting a backslash run, p already points at the next character and this continue statement does another p++, so a byte after an even-length run is never examined. Unescaped # / " after \ pairs can pass validation and be written verbatim; round-trips then lose or truncate data (e.g. a\\#b becomes a\). Please re-evaluate that character when (nbs & 1) == 0 (you could possibly drop the continue or adjust p).

Besides, while here, please clarify and document whether BSDCONF_PUT_UNQUOTED values are literal bytes or pre-escaped text, since the reader runs strunexpand() but the writer does not escape.

543–545

bsdconf_put() always runs the mkstemp/write/fsync/rename path even when every option is CHECK or the value is unchanged. That violates the documented "without modifying the file" contract, bumps mtime, severs hard links, and requires write access to the directory (fails for unprivileged callers on an equal-value check). Please skip the commit when nothing would change; especially when all actions are CHECK. Same root cause seems to make idempotent writes (a=1 when already 1) non-idempotent; sysconf -c is fine because it never calls put, but library consumers are not.

usr.sbin/sysconf/sysconf.c
157

The pre-scan skips the argument after a cluster whose last character is one of the option letters taking an argument. Attached forms like sysconf -f/boot/loader.conf loader rc are mishandled, as here for example, it would treat rc as the target and exec sysrc with the wrong argv.

To my understanding, any -f/-j/-k/-R value ending in A a c d D E e F f i j k l L n N q R v x is affected. Since this exists only to detect the rc pass-through, perhaps we could use real getopt parsing or at least treat attached arguments as self-contained so we don't consume the following argv element.

623–624

The behaviour here may confuse users even though it is correct for each format.

For make/src/src-env, a trailing + on the name selects the make += operator and we replace the whole assignment. So, for instance, an existing CFLAGS+=-g -pipe plus sysconf make CFLAGS+=-O2 becomes CFLAGS+=-O2, dropping -g -pipe.

For every other target, trailing +/- is a list edit and merge_list_requests() appends or strikes words in the current value, so IGNORE_LIST+=c, for example, merges into what is already there.

From maintainer perspective this matches each consumer’s syntax and looks just fine; from user perspective, though, the same CLI spelling has different effects on different targets, easily misused, especially with -q that'd hide the old -> new echo that would otherwise warn them.

I don't know whether we should internally turn it into something well established to avoid such cases and prevent foreseen misuse, but it concerns me a little, to be honest.

1464–1466

Somewhat related to the other comment atop bsdconf_put(), but I recommend consulting opt->result & BSDCONF_VALUE_CHANGED, regardless.

This is mostly a matter of personal preference, so it may be entirely subjective. I just think it would improve the user experience, but I'm happy with whatever you decide.

1775–1776

You've excluded check here, but do_checks() touches no file, so it is read-only like pure reads, right? So it's possibly worth dropping it from the condition here so checks get the same Capsicum path.

kfv: skip no-op put commits; fix verbatim_ok and find_target -f attach

  • bsdconf_put: defer mkstemp/rename until a real edit (CHECK and equal-value SET leave mtime/hardlinks alone; unprivileged equal-value SET no longer needs dir write)
  • bsdconf_verbatim_ok: re-examine the byte after an even-length backslash run (a\\#b was accepted then truncated on re-parse); document BSDCONF_PUT_UNQUOTED as literal file text
  • find_target: attached -fPATH/-Rdir are self-contained (sysconf -f/boot/loader.conf loader rc no longer misidentifies rc)
  • empty-value trim: t > value guard; -c joins the Capsicum path; echo only when VALUE_CHANGED
  • make += vs list-edit left as-is (matches each consumer; already documented in sysconf.8)
dteske added inline comments.
usr.sbin/sysconf/sysconf.c
157

Confirmed — thank you. Attached forms were mishandled exactly as you
described (-f/boot/loader.conf loader rc treated rc as the target).

Took the second path you suggested: walk the cluster like getopt would and
treat an attached argument as self-contained, so only a trailing
option-letter with a separate argv (-f PATH) consumes the next element.
Left the pre-scan hand-rolled rather than calling getopt here, since this
path has to leave argv pristine for the rc → sysrc(8) pass-through
(and we already special-case --help/--version before getopt for the
same reason plain getopt cannot express them).

623–624

Agreed on the UX footgun — same spelling, different effect, and -q hides
the old -> new that would otherwise tip people off.

I'd rather keep the per-consumer semantics than invent a third spelling:
make/src/src-env += is the assignment operator and replaces; elsewhere
+=/-= are sysrc(8)-style list merges. That matches what each file's
reader actually does, and sysconf.8 already calls it out at the
CFLAGS+=-DDEBUG example. Happy to sharpen that wording if you think it
still undersells the surprise; I don't want to change the behavior
itself without a stronger reason.

1775–1776

Yes, do_checks() only compares; it never calls bsdconf_put(). Dropped
!check from the scan_pass() condition so -c gets the same sandbox as
pure reads (still excluding sysctl -d, whose kernel queries the sandbox
would deny).

dteske marked 2 inline comments as done.

Fix one single 80c width issue in a comment; s/emit//

dteske marked an inline comment as not done.Thu, Jul 30, 1:34 AM
dteske added inline comments.
usr.sbin/sysconf/sysconf.c
623–624

Failed += replicated. Working on a fix. For others reading this:

$ sysconf make -a
USE_PACKAGE_DEPENDS: yes
MAKE_JOBS_UNSAFE: yes
DEVELOPER: yes
$ sudo sysconf make CFLAGS+="-g -pipe"
CFLAGS:  -> -g -pipe
$ sysconf make -a
USE_PACKAGE_DEPENDS: yes
MAKE_JOBS_UNSAFE: yes
DEVELOPER: yes
CFLAGS: -g -pipe
$ sudo sysconf make CFLAGS+=-O2
CFLAGS: -g -pipe -> -O2
$ sysconf make -a
USE_PACKAGE_DEPENDS: yes
MAKE_JOBS_UNSAFE: yes
DEVELOPER: yes
CFLAGS: -O2
$ cat /etc/make.conf
# ONLY = do not build missing dependencies from source, just fail
#?USE_PACKAGE_DEPENDS_ONLY=yes

USE_PACKAGE_DEPENDS=yes
MAKE_JOBS_UNSAFE=yes
DEVELOPER=yes
CFLAGS+=-O2

Well that's not going to pass muster. Not one bit. There's actually two ways += can work in the make context, it can either add a new += line because += is supported in make, or we can modify the last assignment (or we can introduce an option to allow the user to choose -- I say we keep it simple and in config formats that support += themselves, like make, we append a new line after the last assignment that uses the += operator).

This also means we need to make sure that we implement support for reading += from confs that support it so that we can accurately report the real value.

Right now, we have a verifiable, reproducible bug which needs addressing that you found.

$ make -V foo

$ sudo sysconf make foo=1
foo:  -> 1
$ make -V foo
1
$ echo 'foo+=2' | sudo tee -a /etc/make.conf
foo+=2
$ make -V foo
1 2
$ tail -n 2 /etc/make.conf
foo=1
foo+=2
$ sysconf make foo
foo: 2

make -V variable gets it right while sysconf make variable gets it wrong.

Good catch.

dteske marked an inline comment as not done.Thu, Jul 30, 1:59 AM
dteske added inline comments.
usr.sbin/sysconf/sysconf.c
623–624

Piling-on to my own thought process, ... holy moly ... I just realized there's some real value-add we're leaving on the table here that should be addressed.

Just how sysconf loader ... is multi-file and addresses how FreeBSD loader gets its values (in the same vein that sysrc's whole purpose is to make sure you can interrogate as well as modify the way the boot process and service sees values in each respective context) sysconf make can (and should) be multi-file as well.

Explained simply, make -V CFLAGS references more than just /etc/make.conf to get the effective value of CFLAGS to compilation with make.

As such, sysconf make ... should reference the same variables. However, I just realized that I ultimately want more than just -v in our implementation (which currently shows only /etc/make.conf and the final value -- at-present, the incorrect value to boot).

Going to have to refactor/rewrite sysconf make to implement (proposed) -V to provide more verbosity -- display all the places that a variable's contents is changed (including appends, since make supports += as a language itself).

dteske marked an inline comment as done.
dteske edited the summary of this revision. (Show Details)

Split manuals/code; CTLTYPE sysctl range checks; make strike + match_line; ATF; -A/docs accuracy

Update since previous revision:

Split the manuals pkg(8)-style so each page has one job:

  • bsdconf(3) is the reading hub; writing moved to bsdconf_put(3); formats/discovery to bsdconf_format(3) (MLINKS follow).
  • sysconf(8) is the shared hub; multi-file policy lives in sysconf-targets(8); each target has its own page (loader, sysctl, make, src, rc, generic).

Frontend and library were also split for maintainability:

  • sysconf(8) is no longer one large .c — assign/make/write/print, resolve/scan, and query paths are separate translation units behind sysconf_priv.h.
  • Statement scan/put helpers moved into bsdconf_stmt.c.
  • src-env is no longer a separate format TU; the src triad and SRC_ENV_CONF / __MAKE_CONF / SRCCONF handling live with the src format / frontend.

API and put semantics:

  • Union gained num64 / u_num64 (BSDCONF_TYPE_INT64 / UINT64); former reserved slots clarified (BSDCONF_TYPE_RESERVED; DATA1..3 remain application-opaque).
  • match_line lets put rewrite/remove a specific physical statement (needed for make += and for sysconf make/src list-strike).
  • bsdconf_put(3) BUGS -> LIMITATIONS: library match_line / make += behavior documented; word-level -= remains sysconf's job.

Real operator anti-footgun for the sysctl target:

  • Before landing a write, validate against the running kernel: unknown OID, read-only, CTLFLAG_TUN (steer to the loader target), and CTLTYPE range checks so a typo or overflow does not poison sysctl.conf (-R skips the live tree checks). Covered by ATF sysctl_oid_range.

Defaults / -A clarifications (docs + examples):

  • Named reads already reflect defaults; -A only widens dump scope (or dumps everything when given alone). Dropped the "folds into sourcing order" and "assignment maps directly to sysctl(8)" wording.
  • Replaced the non-working sysconf loader -A autoboot_delay example (commented-out defaults are not live defaults; we do not teach scraping those) with sysconf loader -A.
  • -l parenthetical corrected; loader.conf chase wording aligned with the loader; miscellaneous man polish (runtime, mdoc Fn nesting).

Tests:

  • New ATF suite under usr.sbin/sysconf/tests (mtree entry added): make += accumulate, make -= strike-last-assign, src triad / -V, src-env rejection, env overrides, attached -f, verbatim backslash, equal-value mtime no-op, sysctl OID range.

Update: make/src WITH_/WITHOUT_ presence knobs

Treat WITH_/WITHOUT_ on the make and src targets as presence knobs
(bsd.mkopt.mk / src.conf(5)): reads show presence, -s sets the empty
assignment form, and WITH_*=no (exact) warns on write and on read like
bsd.mkopt.mk so a bad assignment is caught before an /usr/src build
surfaces it. WITHOUT_MODULES stays a normal module-list directive.

Man pages and ATF updated for the above.