various fixes - #2173
Open
giuseppe wants to merge 5 commits into
Open
Conversation
Only the first byte of the buffer is ever inspected, so a two byte buffer is bigger than needed. Reduce it to a single byte and read sizeof (buf) accordingly. Closes: containers#2160 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 256 byte buffer is much bigger than needed: the code only compares the leading "unconfined" token. Size the buffer to that token plus one byte, which is all the comparison requires. Closes: containers#2161 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous check had a few problems: - a short read (fewer bytes than "unconfined") returned 0, reporting the process as unconfined, when the safe assumption is that it is confined; - a profile named e.g. "unconfined_foo" matched on the first 10 bytes and was reported as unconfined; require the token to be followed by a delimiter (end of data, '\n' or ' '); - the return value was the raw memcmp() result, which can be negative and would be misinterpreted by callers as an error; - the read was not wrapped in TEMP_FAILURE_RETRY, so an EINTR turned into a spurious hard error. Normalize the result to 1 (confined) / 0 (unconfined) and default to confined whenever the token does not match exactly. Closes: containers#2165 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mount_masked_dir() aliased the container-owned private_data->maskdir_fd into a cleanup_close local, which created two owners for the same descriptor. On the fstat() failure path the cleanup handler closed the descriptor while private_data kept the stale number (later double closed at teardown); on the fs_move_mount_to() failure path the descriptor was closed explicitly and then a second time by the cleanup handler, with do_mount() running in between and able to reuse the number. Stop aliasing: use private_data->maskdir_fd directly for the fallible operations and let the cleanup_close local own only the newly created bind mount. private_data->maskdir_fd is replaced only at the end, once no further error is possible, so ownership is never shared. Closes: containers#2170 Closes: containers#2171 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On the fs_move_mount_to() failure path in mount_masked_dir() tmp_err is always NULL, so the "tmpfs fallback" warning always printed "unknown error" and the ternary guarding it always took the false branch. Capture the errno from fs_move_mount_to() into tmp_err so the fallback warning reports the actual reason. Save errno before anything else can clobber it and release any previously stored error first, so tmp_err is never overwritten (and leaked) while holding an allocation. Closes: containers#2172 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
more details in each commit
@eriksjolund PTAL