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

libs: protection: x86_64: drop root requirement for querying #8549

Merged
merged 2 commits into from Dec 4, 2023

Conversation

jodh-intel
Copy link
Contributor

It is no longer necessary to be root to query the guest protection
(TDX) on x86_64 systems, so drop the requirement.

Also update kata-ctl to remove the duplicate, and now redundant, root check.

Fixes: #8548.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

@jodh-intel
Copy link
Contributor Author

/cc @amshinde, @cmaf, @fidencio.

@katacontainersbot katacontainersbot added the size/small Small and simple task label Dec 1, 2023
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

@jodh-intel, just one comment, but overall lgtm.

@@ -13,7 +13,7 @@ use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;

#[cfg(any(target_arch = "s390x", target_arch = "x86_64"))]
#[cfg(any(target_arch = "s390x", target_arch = "powerpc64le"))]
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why powerpc64le was added here?

It seems not related to this commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a comment to the commit. Basically PPC64 needs this import fwics but it was previously missing.

It is no longer necessary to be `root` to query the guest protection
(TDX) on `x86_64` systems, so drop the requirement.

> **Note:**
>
> This change drops the `nix` `Uid` import required for the `root` check.
> But at the same time it adds it for PPC64le since that implementation of
> `available_guest_protection()` needs it and it was previously missing.

Fixes: kata-containers#8548.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Remove the redundant `kata-ctl` `root` check when running the `env`
command. This check duplicated the `GuestProtection` check, and that
check is now no longer necessary anyway.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel added safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test labels Dec 1, 2023
@jodh-intel
Copy link
Contributor Author

/test

Copy link
Contributor

@cmaf cmaf left a comment

Choose a reason for hiding this comment

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

@jodh-intel LGTM.

@jodh-intel jodh-intel added safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test and removed safe-to-test Add to PR after manually reviewing to allow certain extra checks to run ok-to-test labels Dec 4, 2023
@jodh-intel jodh-intel merged commit e4aebb4 into kata-containers:main Dec 4, 2023
275 of 303 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test safe-to-test Add to PR after manually reviewing to allow certain extra checks to run size/small Small and simple task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libs: x86_64: Don't require root to query GuestProtection
5 participants