The man page doesn't even mention errno. It just says that ferror() should
be used to check for errors. Those writes are unlikely to fail, but if they
do, errno might even be 0. Also, we have fflush_and_check() which does
additional paranoia around errno, because we apparently do not trust that
errno will always be set correctly.
Let's consider the following case:
- the direction is down,
- no cached entry,
- the array has 5 entry objects,
- the function test_object() reutns TEST_LEFT for the 1st object,
- the 2nd, 3rd, and 4th objects are broken, so generic_array_bisect_step()
returns TEST_RIGHT for the object.
Then, previously, generic_array_bisect_step() updated the values like the following:
0th: (m = 5, left = 0, right = 4, i = 4) -> (m = 4, left = 0, right = 3, RIGHT)
1st: (m = 4, left = 0, right = 3, i = 1) -> (m = 4, left = 2, right = 3, LEFT)
2nd: (m = 4, left = 2, right = 3, i = 2) -> (m = 2, left = 2, right = 1, RIGHT) <- ouch!!
So, assert(left < right) in generic_array_bisect() was triggered.
See issue #30210.
In such situation, there is no matching entry in the array. By returning
TEST_GOTO_PREVIOUS, generic_array_bisect() handles the result so.
Fixes a bug introduced by ab8f553d1e.
Fixes#30210.
Let's consider the case that the 1st entry in an array is broken, but
n-th entry is valid. Then, if generic_array_get() is called to read
n-th object, the offset of the broken entry is cached by the function.
If generic_array_bisect() is followed, even if the matching entry is
valid, it always fail with -EBADMSG or friends, as the function test the
cached entry at the beginnning. Let's ignore the failure in testing the
cached entry.
Also, field_is_valid(field) already does isempty(field), so drop that as
well.
$ SYSTEMD_LOG_LEVEL=debug journalctl -o verbose -F foo-bar-baz
...
Assertion 'field_is_valid(field)' failed at src/libsystemd/sd-journal/sd-journal.c:2789, function sd_journal_query_unique(). Ignoring.
Failed to query unique data objects: Invalid argument
Don't use assert_runtime() when we get an invalid match string, since
that's a runtime error:
$ SYSTEMD_LOG_LEVEL=debug coredumpctl info =
...
Adding match: =
Assertion 'match_is_valid(data, size)' failed at src/libsystemd/sd-journal/sd-journal.c:240, function sd_journal_add_match(). Ignoring.
Failed to add match "=": Invalid argument
Those functions take a pointer to a timestamp and return a timestamp pointer,
so the reader would be justified to think that those are just getters. Rename
them to avoid confusion.
Currently empty epochs are not sealed. This allows an attacker to truncate
a sealed log and continue it without any problems showing when verifying the
log.
This partially addresses CVE-2023-31438. One way to extend this change to
address CVE-2023-31438 completely, would be to verify that there is exactly
one seal per epoch (and not sealing when the epoch has not ended yet).
the change also adds a journal-file flag: HEADER_COMPATIBLE_SEALED_CONTINUOUS
this flag indicates that a journal file is sealed continuously and decides whether
any missing crypto epochs should trigger a warning or an error.
Previously, OBJECT_UNUSED was used for 'pinning' the mmap cache for an
object. But, OBJECT_UNUSED is also used for reading object when type
cannot be determined before read, e.g. when reading the tail object.
Let's introduce another category for pinning mmap cache, and use it when
we want to temporary pin an object.
- Rename generic_array_bisect_one() -> generic_array_bisect_step(), as there
is also generic_array_bisect_plus_one(), so the original name is confusing.
- Make generic_array_bisect_step() return TEST_GOTO_NEXT or TEST_GOTO_PREVIOUS
when the current array does not contain any matching entries.
- Make generic_array_bisect_step() symmetric with respect to the direction
we are going to, except for the journal corruption handling.
- Make generic_array_bisect_step() gracefully handle journal corruptions,
so the corruption handling in the caller side can be mostly dropped.
- Especially, when the last entry in an array is corrupted, previously
we tried to find a valid entry sequentially from the end of the array,
but now we anyway bisect the array. That should improve performance of
reading corrupted journal files.
- Return earlier when no entry linked to the chained array (n == 0).
- Add many comments.
No behavior change unless journal is corrupted.
This effectively reverts e562f13158.
In the loop of the generic_array_bisect(), the offset of the entry array
object is unchanged, the object is read at the beginning of the loop, and
we do not read any other entry array object. Hence, it is not necessary to
re-read the object every time we use the object.
This is preparation for #28891, which adds a bunch more helpers around
"struct iovec", at which point this really deserves its own .c/.h file.
The idea is that we sooner or later can consider "struct iovec" as an
entirely generic mechanism to reference some binary blob, and is the
go-to type for this purpose whenever we need one.
PAGE_ALIGN() and friends take size_t, while offset is uint64_t.
Let's use macros for uint64_t.
Also, mmap() takes size_t for size. So, let's also use size_t to
calculate a window size.
Prompted by CID#1491286.
When verifying seals produced with forward secure sealing, the verification
currently does not check that old entries are only sealed with the key for
their epoch and not a more recent one. This missing check allows an attacker
to remove seals, and create new ones with the currently available key, and
verify will claim everything is in order, although all entries could have
been modified.
This resolves CVE-2023-31439.
Co-authored-by: Felix Dörre <felix.doerre@kit.edu>
Previously, if the input offset 'p' does not point to an entry object,
the function returns the next of the nearest entry object on
DIRECTION_DOWN, as generic_array_bisect() already returns the nearest
entry object.
If the first call of generic_array_bisect_plus_one() provides the same
offset, then it is not necessary to call the next one, as we already
know the entry object is also liked to the input data object.
Also, this make the function reuse the object returned by
generic_array_bisect_plus_one().
No functional change, just optimization.
Follow-up for ec50313d4e.
The function generic_array_bisect_plus_one() does not read any new data
objects, so the data object is still valid, and not necessary to re-read it.
And MMapFileDescriptor always references MMapCache, hence it is not
necessary Window to have a reference to MMapCache.
This also renames the list fields 'by_fd' -> 'windows', to make them
consistent with the name of the head.
No functional change, just refactoring.
The post operations (attach the found or new window to a context, update
the keep_always flag, and calculate the address) in three steps are
equivalent, and the two searching logics are quite simple.
Let's merge them to decrease the total lines.
No functional change, just refactoring.
This reverts the following commits.
- a1640191b4
- 231741d355
These were done by my misunderstanding of the mmap cache behavior.
Also, this updates the comments added by
df04b9ed86.
Yu Watanabe
Mike Yuan
Zbigniew Jędrzejewski-Szmek
Frantisek Sumsal
Felix Dörre
Luca Boccassi
Lennart Poettering
Daan De Meyer
felixdoerreandFelix Dörre