Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virt: fix container detection #25941

Merged
merged 1 commit into from Jan 5, 2023
Merged

Conversation

bentiss
Copy link
Contributor

@bentiss bentiss commented Jan 5, 2023

Commit 1b86c7c ("virt: make virtualization enum a named type") made the conversion from if (!r) to if (v != VIRTUALIZATION_NONE).

However, the initial test was meaning "if r is null", IOW "if r IS VIRTUALIZATION_NONE).

The test is wrong and this can lead to false detection of the container environment (when calling systemctl exit).

For example, https://gitlab.freedesktop.org/whot/libevdev/-/jobs/34207974 is calling systemctl exit 0, and systemd terminates with the exit code 130.

Fixing that typo makes systemctl exit 0 returns 0.

Fixes: 1b86c7c.

This was detected in Fedora 37. Fedora 36 was fine as it is relying on systemd v250 which doesn't expose the bug.

Cc: @whot

Commit 1b86c7c ("virt: make virtualization enum a named type")
made the conversion from `if (!r)` to `if (v != VIRTUALIZATION_NONE)`.

However, the initial test was meaning "if r is null", IOW "if r IS
`VIRTUALIZATION_NONE`).

The test is wrong and this can lead to false detection of the container
environment (when calling `systemctl exit`).

For example, https://gitlab.freedesktop.org/whot/libevdev/-/jobs/34207974
is calling `systemctl exit 0`, and systemd terminates with the exit code
`130`.

Fixing that typo makes `systemctl exit 0` returns `0`.

Fixes: 1b86c7c.
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Jan 5, 2023
Copy link
Member

@bluca bluca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch, good catch

@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed detect-virt needs-stable-backport and removed please-review PR is ready for (re-)review by a maintainer labels Jan 5, 2023
@poettering
Copy link
Member

lgtm

@poettering poettering merged commit a91078b into systemd:main Jan 5, 2023
@bentiss bentiss deleted the fix-container-env branch January 6, 2023 07:53
@keszybz keszybz removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed needs-stable-backport labels Feb 10, 2023
mmstick pushed a commit to pop-os/libinput that referenced this pull request Mar 30, 2023
This was delayed by systemd/systemd#25941 which
is now available in F37 as of v251.11

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
dangel101 pushed a commit to dangel101/libevdev that referenced this pull request Sep 14, 2023
The F37 update was delayed by systemd/systemd#25941 which
is now available in F37 as of v251.11

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants