-
Notifications
You must be signed in to change notification settings - Fork 835
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
PVF: fix detection of unshare-and-change-root security capability #2304
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ use polkadot_node_core_pvf_common::{ | |
error::{PrepareError, PrepareResult}, | ||
pvf::PvfPrepData, | ||
}; | ||
use polkadot_node_subsystem::SubsystemResult; | ||
use polkadot_parachain_primitives::primitives::ValidationResult; | ||
use std::{ | ||
collections::HashMap, | ||
|
@@ -203,11 +204,14 @@ impl Config { | |
/// The future should not return normally but if it does then that indicates an unrecoverable error. | ||
/// In that case all pending requests will be canceled, dropping the result senders and new ones | ||
/// will be rejected. | ||
pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Future<Output = ()>) { | ||
pub async fn start( | ||
config: Config, | ||
metrics: Metrics, | ||
) -> SubsystemResult<(ValidationHost, impl Future<Output = ()>)> { | ||
gum::debug!(target: LOG_TARGET, ?config, "starting PVF validation host"); | ||
|
||
// Run checks for supported security features once per host startup. Warn here if not enabled. | ||
let security_status = security::check_security_status(&config).await; | ||
let security_status = security::check_security_status(&config).await?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait a second. This looks like it does not actually resolve the issue. If we can't check, we can now no longer do any validation, despite being not yet strict in enforcing security? Am I missing something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the only possible error should never happen (https://github.com/paritytech/polkadot-sdk/blob/2e2a75ff81/polkadot/node/core/pvf/src/worker_intf.rs#L189-L196). But I could have done this better. Thanks! |
||
|
||
let (to_host_tx, to_host_rx) = mpsc::channel(10); | ||
|
||
|
@@ -273,7 +277,7 @@ pub async fn start(config: Config, metrics: Metrics) -> (ValidationHost, impl Fu | |
}; | ||
}; | ||
|
||
(validation_host, task) | ||
Ok((validation_host, task)) | ||
} | ||
|
||
/// A mapping from an artifact ID which is in preparation state to the list of pending execution | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already was going to add
-> SubsystemResult
here in another PR soon.