So far BLOCKDEV_LIST_IGNORE_VIRTUAL filtered out everything below
/sys/devices/virtual/, which includes loopback devices. Loopback devices
are frequently used as stand-ins for real disks though (in tests, and
when installing to disk images), hence it's useful to be able to control
them independently of the other virtual devices (dm, md, zram, …).
Introduce a separate BLOCKDEV_LIST_IGNORE_LOOP flag for loopback
devices, and make BLOCKDEV_LIST_IGNORE_VIRTUAL only cover the non-loop
virtual devices. The only existing user (systemd-report) is updated to
pass both flags, so that its behaviour stays unchanged.
When PCR is extended during a TPM2 operation, the internal counter of
the TPM2 will detect a new extension. systemd already retry this
operation several times, but sometimes the report will be an error, and
will refer to the wrong PCR.
This situation happends when IMA is enabled (when secure boot is active,
for example). PCR 10 will receive an extension for each new kernel
module that is loaded, logging in the journal errors that are recovered
later after a subsequent retry.
We replace the implicit single callback and wait-future indirection with
the usual slot design so independent fibers and future groups can track
the same future. We also opt to always have callbacks dispatch asynchronously,
including when they are registered after the future has already resolved.
We now require futures to be resolved before their final unref rather than
cancelling them implicitly. The idea is to catch bugs rather than leak futures.
A future should always be cancelled and awaited before it is cleaned up,
otherwise resources might leak if the stack is not unwound properly. We make
exceptions for futures that haven't started executing yet as there's no risk
of leaking resources there since there's no stack to unwind.
One issue with awaiting futures is that sd_fiber_await() returned both
errors and the future's result via the function's return value, meaning it
was impossible to distinguish future failures from interrupted waits (or
waits that failed for whatever other reason). This is generally not an issue
except in the cleanup functions, where we have to retry on interrupted waits
(the best example is a parent fiber waiting for a future being cancelled
itself, which should cancel the future it's waiting for and retry the wait).
So we introduce fiber_wait() which distinguishes between both values via
return value and output argument so the two can be distinguished from each
other.
Cleanup always consumes the caller's reference, including on invalid self or
cross-loop waits. The existing assertion still forbids dropping the last
reference to an unresolved future. The installed header documents this rule.
We introduce a new RUNNING fiber state to distinguish executing fibers from
non-executing fibers. We replace CANCELLED with queued resume results and
preserve interruptions across ordinary wakeups.
We add future groups with wait-all, wait-any and ignore-errors policies to
wait for multiple futures at once and migrate various existing call sites over
to them. Group size is a direct size_t query, treating NULL as an empty group.
Document all four policy combinations, including that IGNORE_ERRORS suppresses
fail-fast and parent cancellation without discarding the group's error result.
We also add defer futures and migrate bus, varlink, QMP and fiber I/O callers to
use them. Bus and QMP futures report -ENOPKG when no event loop is attached,
rather than passing NULL to sd_future_new() and returning -EINVAL.
Parent-await suppression is derived from the active wait's callback slot, so
groups use sd_fiber_await() without a separate await API or additional state.
Waits, group children and captured parents must use the same event loop.
Groups created on another loop have no parent, and a group releases its
parent reference when that fiber finishes.
Resolve SD_EVENT_DEFAULT in sd_future_new() so futures retain the actual
loop. Match group parents against that resolved loop as well; an unavailable
default returns -ENOPKG without creating an event loop implicitly.
Keep the public await entry point in fiber.c, sharing the slot-based wait with
cancellation cleanup through future-internal.h. Keep event/future integration
in event-future.c/h: priority inheritance uses the public fiber accessor, and
sd_future_new_defer() lives alongside the other event-backed constructors.
This avoids sd-event depending on sd-future private headers and removes the
redundant private defer wrapper.
Assert the non-failing slot-disabling and active-loop invariants instead of
returning early without completing callback dispatch.
Regression tests cover group error propagation and NULL-safe size queries,
interrupted waits, partial add-many rollback, groups outliving their parent,
and cancellation or timeout during nested event-loop waits, including
event-loop and descriptor cleanup. They also cover cross-loop parent isolation,
reference release on invalid cleanup calls, mixed WAIT_ANY outcomes, failed
child cancellation and policy changes on resolved groups. Additional tests
verify failed group-priority updates, slot priority during dispatch and exit,
and cancellation, timeout and spurious wakeups during bus and QMP calls. Late
replies must leave subsequent calls unaffected. Default-event tests cover
missing-loop errors, reference lifetime and parent capture by constructors.
Unattached bus and QMP clients are covered by missing-event-loop regressions.
Constructor tests verify inherited and default priorities for I/O, defer and
timer futures, plus defer argument validation and missing-default errors.
Signed-off-by: Daan De Meyer <daan@amutable.com>
Three independent fixes to code on `journalctl`'s per-entry read and
output path. Each of them removes work that was never needed, rather
than trading away anything. There is no behaviour change: the output is
byte for byte identical for every format except `-o json`, where only
the order of the keys within each object can shift, which is already
nondeterministic between two runs of the unpatched binary (see the
correctness section below). Nothing about the formats, tunables or
public API changes.
Measured on a 730 MB journal spread over 100 files (roughly 2M entries):
| command | before | after | |
|---|---:|---:|---:|
| `journalctl -o cat` | 1.93s | 0.77s | **2.51x** |
| `journalctl --grep=<no match>` | 1.97s | 0.82s | **2.40x** |
| `journalctl -o export` | 4.06s | 1.73s | **2.35x** |
| `journalctl -n 100000 -o short` | 0.86s | 0.43s | **2.00x** |
| `journalctl -o json -n 200000` | 5.85s | 3.50s | 1.67x |
| `journalctl -o short` (the default) | 3.32s | 1.96s | **1.69x** |
| `journalctl -o verbose` | 5.99s | 3.61s | 1.66x |
### 1. `utf8_is_printable_newline()` decoded every character twice
`utf8_encoded_valid_unichar()` decodes the character internally in order
to validate it, and then throws the codepoint away. The caller
immediately called `utf8_encoded_to_unichar()` again to get the very
same codepoint back. So for every byte of every field printed, the UTF-8
decoder ran twice, only to answer whether the byte was a control
character.
This is fixed where it originates rather than at the call site:
`utf8_encoded_valid_unichar_full()` optionally returns the character it
had to decode anyway, so a caller that wants both the validity and the
value decodes once. On top of that, eight bytes are tested at once for
plain printable ASCII, which is what almost all journal content is. A
word that does not qualify falls through to the per character path
unchanged, so tabs, newlines and multibyte sequences behave exactly as
before.
On its own, against unmodified main: `-o export` 4.02s to 2.89s.
### 2. sd-journal read the clock once per journal file, per entry
`sd_journal_next()` sweeps every open journal file to find the next
entry, and `next_beyond_location()` rate limits its tail timestamp
refresh per file. Since `ratelimit_below()` reads the clock itself, a
journal spread over a hundred files meant a hundred `clock_gettime()`
calls for every single entry iterated, nearly all of them only to
conclude that the one second interval had not elapsed yet.
This was the single largest cost in `journalctl`. In a profile of `-o
export` it was over a fifth of the entire run, spent deciding not to do
the thing it was guarding.
Add `ratelimit_below_at()`, which takes the timestamp from the caller,
and read the clock once per sweep instead. `ratelimit_below()` keeps its
behaviour and becomes a wrapper.
The timestamp is now sampled once per sweep rather than once per file.
The interval being rate limited is one second and a sweep takes
microseconds, so this makes no practical difference.
On its own: `-o export` 4.02s to 2.93s, and `-o short` 3.30s to 2.14s.
This is the largest single contributor for the default output mode.
### 3. The JSON output built a fresh, unsized hashmap for every entry
`journal_entry_to_json()` creates an empty hashmap per entry and fills
it with that entry's fields, of which there are typically around twenty.
Because it starts empty it has to grow several times per entry, and
every growth rehashes everything inserted so far. `-o json` spent about
a quarter of its time in siphash, a third of that under
`resize_buckets()`.
Reserving room up front is a one line change.
On its own: `-o json -n 200000` 5.71s to 4.64s.
This adds 'add' boolean parameter to sd_nfnl_nft_message_new_table() and
friends, so that we can remove nftables table and so on. In
test-firewall-util, before and after running test cases, remove the
garbage nftables test table.
Split out of https://github.com/systemd/systemd/pull/42983
## Problem
`dns_scope_announce()` put the entire zone into a single reply packet —
for DNS-SD that is every PTR/SRV/TXT record of every published service.
With a few hundred services that grows far past the interface MTU, and
past what RFC 6762 section 17 allows: a Multicast DNS packet larger than
the MTU is sent using fragments, and such a packet "MUST NOT contain
more than one resource record".
## Fix
Pack an announcement into as many MTU-sized packets as needed. A record
that does not fit the MTU on its own is emitted alone in an oversized
packet relying on IP fragmentation — the case section 17 explicitly
allows — bounded by its hard 9000-byte ceiling including headers, which
also caps jumbo-frame links.
Without a usable interface MTU (no link on the scope, or `IFLA_MTU` not
read yet) packets are cut to the largest datagram a host of the family
must accept unfragmented — RFC 791's 576 bytes for IPv4, RFC 8200
section 5's 1280-byte minimum link MTU for IPv6 — rather than to that
ceiling, so an announcement is never fragmented merely because the MTU
was not known yet, and the lone-oversized-record path stays reachable.
A record that cannot be encoded at any size (an rdata past 64KiB, a
character string past 255 bytes) is skipped like one too large for the
ceiling rather than failing the whole announcement:
`dns_packet_append_rr()` rolls the packet back on every failure, so its
neighbours are unaffected either way.
The packing lives in `resolved-mdns.c` as
`mdns_announcement_packetize()`, taking an answer and the two size
bounds and returning finalized packets, so it is testable without a
scope, a link or a socket. Emission is best effort per packet — a failed
send no longer withholds the rest — and each pass logs the packet and
record counts it actually put on the wire.
The default attestation key for the TPM2 report signer is currently
created with an empty policy but with the adminWithPolicy attribute set.
This makes it impossible to authorize use of the key for the ADMIN role,
which is required by TPM2_Certify.
This drops the adminWithPolicy attribute so that it can be authorized
using its (empty) authorization value. This is appropriate for local
style attestation keys ("LAKs"). I've been working on an interface for
managing attestation keys which will support creating an IAK, and this
will have both the adminWithPolicy attribute set and a policy defined as
specified in the TCG "TPM 2.0 Keys for Device Identity and Attestation"
spec.
journal_entry_to_json() builds a fresh hashmap for every entry and then fills
it with that entry's fields, of which there are typically around twenty. Since
the hashmap starts out empty it has to grow several times per entry, and every
growth rehashes everything inserted so far. journalctl -o json spent about a
quarter of its time in siphash, a third of that under resize_buckets().
Reserve room up front. On a 730M journal spread over 100 files,
journalctl -o json -n 200000 goes from 5.71s to 4.64s.
systemd has two UTF-8 decoders, and the console width path uses the
permissive one.
`utf8_encoded_to_unichar()` performs no overlong check, no
`unichar_is_valid()` check, and still accepts the obsolete five and six
byte forms. Those checks live in the wrapper
`utf8_encoded_valid_unichar()`, which is what `utf8_is_valid()` and
`ellipsize_mem()` use. `utf8_char_console_width()` called the bare
decoder directly.
Checking both against `utf8_is_valid_n()` over all 4,244,897,280 NUL
free byte strings of length one to four, which is the entire UTF-8
encoding space:
| decoder | disagreements with `utf8_is_valid_n()` |
| --- | --- |
| `utf8_encoded_valid_unichar()` | 0 |
| `utf8_encoded_to_unichar()` | 8,835,838, always more permissive |
So `utf8_console_width()` returned a finite width for strings
`utf8_is_valid()` rejects. `format-table.c` measures a cell with
`utf8_console_width()` and, when it does not fit, hands it to
`ellipsize()`, which validates strictly and returns NULL. That is
reported as `-ENOMEM`:
```
lead surrogate valid_utf8=0 utf8_console_width=25 ellipsize=NULL
-> table_format FAILS: Cannot allocate memory
```
A surrogate, overlong, noncharacter or five byte sequence in any cell
failed the whole table with a bogus out of memory error, and only once
that column was narrow enough to need truncation, so whether it happened
depended on the width of the user's terminal. At other widths the
invalid sequence was printed to the terminal instead. Invalid cells are
now refused consistently with `-EINVAL`, which is what the sequences
`utf8_encoded_to_unichar()` already rejected always produced.
The second commit fixes a related disagreement. `ellipsize_mem()` and
`align_string_mem()` open coded character width as `unichar_iswide(c) ?
2 : 1`, while `unichar_console_width()` counts a tab as eight cells.
`format-table.c` uses both: it finds a field too wide with
`utf8_console_width()`, then asks `ellipsize()` to fit that many
columns, and `ellipsize()` thought a tab was one cell, so the result
still overflowed the column that the measurement had just asked it to
fit. A table pinned to `table_set_width(20)` emitted lines of 25, 44 and
39 columns. This uses `unichar_console_width()`, which was split out for
exactly this purpose in 5c0d4b5061.
Both commits add tests that fail before the change and pass after. The
full unit suite is otherwise unaffected.
dns_scope_announce() put the entire zone -- for DNS-SD every PTR/SRV/TXT
record of every published service -- into a single reply packet. With a
few hundred published services that grows far beyond the interface MTU,
and beyond what RFC 6762 section 17 permits: a packet larger than the
MTU is sent using fragments, and such a packet "MUST NOT contain more
than one resource record".
Pack the announcement into as many MTU-sized packets as needed instead.
A single record that does not fit the MTU by itself is emitted alone in
an oversized packet, relying on IP fragmentation, which the same section
explicitly allows for that case -- bounded by its hard 9000-byte
ceiling, headers included, which also caps jumbo-frame links. While the
interface MTU is not known yet, packets are cut to the datagram size
every host of the family must be able to accept -- RFC 791 section
3.1's 576 octets for IPv4, whole or reassembled, and RFC 8200 section
5's 1280-byte minimum link MTU for IPv6, which no link fragments --
rather than to that ceiling, so an announcement is not packed for
deliberate fragmentation merely because the MTU was not read yet.
A record that cannot be encoded at any size -- a character string past
255 bytes, say -- is skipped like one too large for the ceiling, rather
than failing the whole announcement: dns_packet_append_rr() rolls the
packet back on every failure, so its neighbours are unaffected either
way. The packetizer returns how many it dropped and the scope warns on
it, since nothing else would distinguish an announcement that carried
everything from one that did not -- and for a withdrawal that matters:
the caller drops the records from the zone either way, so one skipped
here is never withdrawn at all and peers keep it for its full TTL.
The packing itself lives in resolved-mdns.c as mdns_announcement_packetize(),
taking an answer and the two size bounds and returning finalized packets,
so it can be tested without a scope, a link or a socket.
Emission is best effort per packet -- a failed send no longer withholds
the announcement's remaining packets -- and each pass logs the packet and
record counts it actually put on the wire. Each packet takes its own
token from the scope's multicast rate limit: that limit bounds datagrams
on the wire, and an announcement that would exhaust its burst is the
very flood it exists to cap.
dns_scope_send_goodbye(), the other unsolicited-response emitter, goes
through the same packetizer: a withdrawal of many records had the same
oversized-packet problem, and a single record failing to append
abandoned the whole withdrawal. Its return value follows the emitter's
now, so a withdrawal split across packets reports an error when any one
of them fails to go out.
tpm2_nvpcr_initialize() already retries the initializing write when
tpm2_extend_nvpcr_nv_index() reports that PCR values changed under its
feet, which the comment there attributes to IMA measurements performed
by the kernel. The policy session that write needs is opened in the
same loop, and a PCR extended while that policy is submitted makes
tpm2_policy_pcr() return -EUCLEAN, which was propagated as a hard
failure instead.
That is the same transient condition observed one step earlier, so let
the loop absorb it too.
Signed-off-by: Alberto Planas <aplanas@suse.com>
tpm2_policy_pcr() returns -EUCLEAN when the TPM's global PCR update
counter moved while the policy was being submitted, not when the
recorded PCR values disagree with the current state. Logging "Either
the system has been tampered with or the provided policy is incorrect"
at error level is therefore wrong, and it is noisy too: the condition is
transient and callers are expected to restart the policy session.
Downgrade both occurrences to debug and describe what actually happened,
and document the contract at the declarations.
Signed-off-by: Alberto Planas <aplanas@suse.com>
tpm2_load_pcr_signature() propagates the JSON parser's errno, and the
parser reports a string that is not valid UTF-8 as -EUCLEAN. With
EUCLEAN now mapping to io.systemd.Credentials.PCRsKeptChanging, such a
file would be reported as if a PCR were being extended continuously.
Translate it where it originates, so that the errno stays single valued
for every caller of tpm2_load_pcr_signature() and not only for the creds
one: a signature file that cannot be parsed is -EBADMSG, whichever way
it is malformed. A syntax error used to come out as -EINVAL, which
matched nothing on the way out either.
Signed-off-by: Alberto Planas <aplanas@suse.com>
io.systemd.Credentials.UnexpectedPCRState used to be mapped to EUCLEAN,
which now means "a PCR kept being extended while unsealing" rather than
"the PCR state does not match the policy". Map it to EPERM instead,
which is one of the errnos ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR() covers,
and introduce a separate PCRsKeptChanging error for EUCLEAN.
Also report the condition explicitly in decrypt_credential_and_warn(),
like the other TPM2 unseal callers do.
Signed-off-by: Alberto Planas <aplanas@suse.com>
tpm2_unseal() returns -EUCLEAN specifically when a PCR kept being
extended while unsealing, but the unseal callers no longer catch it
via ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR() and would fall back to the
generic "Failed to unseal secret using TPM2" message.
Give it a message of its own instead, so that the log points at the
actual cause instead of suggesting the system was tampered with.
Signed-off-by: Alberto Planas <aplanas@suse.com>
So far tpm2_unseal() used -EUCLEAN together with the errors that
signal that the PCR state does not match the policy. That is
misleading: -EUCLEAN (and TPM2_RC_PCR_CHANGED) is only ever generated
when the TPM's global PCR update counter moves while we are building the
policy or unsealing, i.e. when something on the system extends a PCR
concurrentlyi (usually IMA extending PCR 10). It says nothing about
whether the policy itself matches.
Until now the retry loop only recognized those conditions while retries
were left (i > 0), and once the budget was exhausted the last iteration
fell through to the generic paths, reporting either an unrelated policy
error or -ENOTRECOVERABLE. Handle the last iteration explicitly instead,
and return -EUCLEAN with a message that names the actual problem.
Drop -EUCLEAN from ERRNO_IS_NEG_TPM2_UNSEAL_BAD_PCR(), it is not a "bad
PCR state" error.
Signed-off-by: Alberto Planas <aplanas@suse.com>
ellipsize_mem() and align_string_mem() open coded the width of a character as
unichar_iswide(c) ? 2 : 1, but unichar_console_width() counts a tab as eight
cells. The two disagreed, and format-table.c uses both: it decides a field is too
wide with utf8_console_width(), then asks ellipsize() to fit it into that many
columns, and ellipsize() believed a tab was one cell. The ellipsized cell then
still overflowed the column, measured by the very function that had just asked
for it to be truncated. A table pinned to table_set_width(20) emitted lines of
25, 44 and 39 columns. align_string_mem() undercounted the same way and so padded
already overlong cells further.
Use unichar_console_width(), split out for this purpose in 5c0d4b5061. While at
it, let align_string_mem() decode with utf8_char_console_width() as well, so that
the second pass shares both the width model and the validity check with the
measuring pass, instead of keeping a more permissive decoder of its own around
for the next caller to trip over. Its "count invalid chars as 1" fallback now
skips the whole rejected sequence rather than one byte, so that the continuation
bytes are not charged a cell each of their own; the branch should be unreachable
anyway, since our caller only gets there after utf8_console_width() succeeded on
the same string.
Note that unichar_console_width() charges a tab a flat eight cells, while a
terminal advances to the next multiple of eight, so this is deliberately an upper
bound: the two agree only when the tab itself starts on a multiple of eight. A
per character helper cannot do better, because a tab's width is a function of the
column it starts in, and neither ellipsize_mem() nor align_string_mem() is told
what column its string starts at. Overestimating is the safe direction: it can
leave a column padded short, but it cannot overflow one.
ascii_ellipsize_mem() equates bytes with character cells, which holds for ASCII
except for the tab, so keep tabs off that fast path. Like the ANSI sequence case
just above it, such strings now take the generic path, which means they get the
Unicode ellipsis and one reserved cell rather than "..." and three, even in a non
UTF-8 locale.
So far we didn't limit the number of jumps when decompressing a DNS
packet. This can lead to some extra work with specially crafted packets
that send a staircase of thousands of descending compression pointers,
which can then be amplified by referencing such staircase from thousands
of names.
Given that legitimate DNS packets need only a handful of jumps in the
compression chains, let's limit the number of jumps per a single name to
256 (the same limit as, for example, Unbound currently has).
Reported by opensec-intelligence (OpenSec).
The _const_ attribute can be only used for functions.
Fixes the following warnings on Fedora rawhide:
```
../src/core/namespace.h:260:75: warning: '__const__' attribute only applies to functions [-Wignored-attributes]
260 | char* bpf_delegate_to_string(uint64_t u, const char * (*parser)(uint64_t) _const_);
| ^
../src/basic/../fundamental/macro.h:80:32: note: expanded from macro '_const_'
80 | #define _const_ __attribute__((__const__))
| ^
In file included from ../src/core/unit.c:24:
../src/shared/condition.h:87:63: warning: '__const__' attribute only applies to functions [-Wignored-attributes]
87 | typedef const char* (*condition_to_string_t)(ConditionType t) _const_;
| ^
../src/basic/../fundamental/macro.h:80:32: note: expanded from macro '_const_'
80 | #define _const_ __attribute__((__const__))
| ^
```
Similar as the previous commit: the field was renamed by accident, and
we should revert to the old name to not break compat needlessly, and
because the old name actually made sense, given the underlying .transfer
file knob is AppStream=
The "isEnabled" name was changed from "enabled" post v261 by accident
(because a6f7ded859 replaced json
formatting via table formatter to a native json generator which was
introduced for varlink, which changed the names). THis was later made
official in #43603, but it should rather have changed things back to the
original name, in particular as the underling .feature file field
actually is called "Enabled=" and not "IsEnabled=".
We really should take care to generate the same names for Varlink JSON
data as for the data returned by the systemd-sysupdate cli.
a6f7ded859 ensured that, so that's great.
But at the same time I think the varlink names should be adjusted to the
old names, since varlink support for this was just added in this cycle.
Given that v262 is not tagged yet, let's now revert to the old name.
For re-use in systemd-gpt-auto-generator. This also makes it generic
across sysext/confext - no planned use for this, but to avoid confusion
later when someone tries to use it on a confext.
Encrypted credentials produced by systemd-creds are stored as Base64.
Decode them before decrypting in read_credential_with_decryption(), while
leaving plaintext credentials unchanged and retaining their precedence.
Bound the decoded encrypted input and report oversized credentials
explicitly. Match the plaintext reader's string semantics by rejecting
embedded NUL bytes when no size output is requested, while preserving
binary payloads for callers that request their size.
Add regression coverage for missing and malformed credentials, plaintext
precedence, decoded size boundaries, single-line and wrapped Base64,
empty strings, and binary data. Keep test outputs and host secrets scoped
to private fixtures.
Fixes#43621
The feature JSON payload switched to `id`, `isEnabled`, and `appstream`,
and made `documentation` an array of URLs.
`updatectl` still parsed legacy table-derived names and a single URL.
This broke feature JSON parsing and made output inconsistent.
Features without documentation could also cause a null dereference.
Align `updatectl` with the current schema and preserve all
documentation URLs across sysupdate, D-Bus, and Varlink payloads.
Propagate condition evaluation errors through `condition_test_list()`
and `feature_is_suggested()`, with consistent JSON error handling and
tests.
The `suggested` field is now optional and always a boolean when
present; it is omitted rather than set to an error string when
SuggestOn*= conditions cannot be evaluated.
Add `suggested` metadata and document the field transition.
Follow-up for 3e34ccf6df.
Follow-up for 75a247d4de.
Follow-up for 40e727c93b.
Fixes#43603
Make condition_test_list() return negative errno on condition
evaluation errors instead of coercing them to false. This allows
callers to distinguish between a condition that evaluated to false
and a condition that failed to evaluate.
Callers that only need a boolean result explicitly check for a
positive return value, preserving their current behavior.
Add comprehensive unit tests covering error propagation with trigger
and non-trigger conditions. Tests use uname_architecture() to get the
runtime architecture instead of native_architecture(), ensuring they
work correctly in environments where binaries run under architecture
emulation.
Standard Linux POSIX ACL tag values are 0x00, 0x01, 0x02, 0x04, 0x08,
0x10 and 0x20. ELEMENTSOF(tag_map) evaluates to 33, which is one past
the last valid index 32.
dns_packet_append_name() appends a name's labels and then the root byte
that terminates it. A failure on that last byte returned directly,
leaving both the labels on the packet and the compression entries this
call had added -- entries naming offsets the caller then truncates away.
Take the same rollback path as every other failure in the function.
Independent of the map invariant fixed before it: this is reachable
whether or not any offset exceeded the pointer range.
dns_packet_append_name() added every name it appends to the packet's
compression map, including names written at offsets >= 0x4000 that an
RFC 1035 compression pointer (14 bits) can never reference. Such
entries are not just useless -- when the same name occurs again, the
lookup finds it, the pointer cannot be emitted, the code falls through
to appending the labels again, and the re-insertion into the map then
fails the whole packet build with hashmap_ensure_put() returning
-EEXIST.
This is easily triggered with mDNS: an unsolicited DNS-SD announcement
covering a zone with a few hundred services exceeds 16KiB, and
dns_scope_announce() then fails with "Failed to build reply packet:
File exists".
This lifts the failure only: dns_scope_announce() still builds the whole
zone into one packet and hands it to the socket as a single datagram,
fragmented as the link requires. Splitting announcements across
MTU-sized packets, as RFC 6762 section 17 asks, is done separately.
Only insert a name into the map if its offset is pointer-expressible,
making "mapped implies referenceable" a map invariant. Re-occurrences of
out-of-range names are emitted as labels again -- the only encoding RFC
1035 section 4.1.4 permits in that case -- still compressed from the
first suffix that is mapped in range.
The lookup restates that range as control flow rather than asserting it.
Under -Db_ndebug=true, which CI builds, assert() is
__builtin_unreachable(), so an out-of-range entry would put 0xC000 | n
truncated to 14 bits on the wire -- a pointer to an arbitrary earlier
offset -- where falling through emits the labels instead. This layer is
shared with unicast DNS, DoT and LLMNR.
The pointer's tag bits and its offset bound are now named once, in
dns-def.h next to the other wire-format constants rather than in
dns-packet.h, since the format is RFC 1035's and not this packet
abstraction's -- dhcp-message.c open-codes the same two bits to parse
compressed names out of DHCP options. Compile-time checks pin that the
two partition the 16-bit field and that the tag is its top two bits,
which is what lets the reader take it from the first octet alone.
The offset's other bound becomes control flow along with it: it was an
assert(), which under -Db_ndebug=true is no check at all, for the bound
whose violation puts a pointer to the wrong bytes on the wire. It is
also tightened to the start of the name being appended rather than the
end of the packet, which is the "prior occurrence" RFC 1035 section
4.1.4 allows a pointer to address and what dns_packet_read_name()
enforces on the way in.
Add regression tests, including one through dns_packet_append_rr() --
the path the reported failure came from, where the name append is nested
inside the rdlength backpatch and two further rollbacks -- and one for
the reader's label-type dispatch, which the renamed constants touch.
Lennart Poettering
Luca Boccassi
Daan De Meyer
Yu Watanabe
Chris Coulson
Armaan Sandhu
Roman Perehonchuk
Christian Glombek
Alberto Planas
Georg Müller
Frantisek Sumsal
Zbigniew Jędrzejewski-Szmek
zouge666
Kai Wohlfahrt
Siteshwar Vashisht
Totoro
dongshengyuan