Skip to content

Commit 3faf393

Browse files
SvenGrootSvenGroot
authored
vmbus_client: don't request confidential channel support. (#917)
The vmbus client requested all feature flags, which includes confidential channels, which it doesn't actually support. This was harmless, as the host cannot support confidential channels, but it would cause the host to log a warning that unsupported feature flags were requested.
1 parent edc9bc1 commit 3faf393

File tree

6 files changed

+50
-40
lines changed

6 files changed

+50
-40
lines changed

‎vm/devices/vmbus/vmbus_client/src/lib.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ use zerocopy::KnownLayout;
5858
const SINT: u8 = 2;
5959
const VTL: u8 = 0;
6060
const SUPPORTED_VERSIONS: &[Version] = &[Version::Iron, Version::Copper];
61-
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::all();
61+
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::new()
62+
.with_guest_specified_signal_parameters(true)
63+
.with_channel_interrupt_redirection(true)
64+
.with_modify_connection(true)
65+
.with_client_id(true);
6266

6367
/// The client interface synic events.
6468
pub trait SynicEventClient: Send + Sync {
@@ -1842,13 +1846,13 @@ mod tests {
18421846
padding: 0,
18431847
selected_version_or_connection_id: 0,
18441848
},
1845-
supported_features: FeatureFlags::all().into(),
1849+
supported_features: SUPPORTED_FEATURE_FLAGS.into(),
18461850
},
18471851
));
18481852

18491853
let version = recv.await.unwrap().unwrap();
18501854
assert_eq!(version.version, Version::Copper);
1851-
assert_eq!(version.feature_flags, FeatureFlags::all());
1855+
assert_eq!(version.feature_flags, SUPPORTED_FEATURE_FLAGS);
18521856
}
18531857

18541858
async fn get_channel(&mut self, client: &mut VmbusClient) -> OfferInfo {
@@ -2009,7 +2013,7 @@ mod tests {
20092013
interrupt_page_or_target_info: TargetInfo::new()
20102014
.with_sint(2)
20112015
.with_vtl(0)
2012-
.with_feature_flags(FeatureFlags::all().into())
2016+
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
20132017
.into(),
20142018
parent_to_child_monitor_page_gpa: 0,
20152019
child_to_parent_monitor_page_gpa: 0,
@@ -2036,7 +2040,7 @@ mod tests {
20362040
interrupt_page_or_target_info: TargetInfo::new()
20372041
.with_sint(2)
20382042
.with_vtl(0)
2039-
.with_feature_flags(FeatureFlags::all().into())
2043+
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
20402044
.into(),
20412045
parent_to_child_monitor_page_gpa: 0,
20422046
child_to_parent_monitor_page_gpa: 0,
@@ -2054,14 +2058,14 @@ mod tests {
20542058
padding: 0,
20552059
selected_version_or_connection_id: 0,
20562060
},
2057-
supported_features: FeatureFlags::all().into_bits(),
2061+
supported_features: SUPPORTED_FEATURE_FLAGS.into_bits(),
20582062
},
20592063
));
20602064

20612065
let version = recv.await.unwrap().unwrap();
20622066

20632067
assert_eq!(version.version, Version::Copper);
2064-
assert_eq!(version.feature_flags, FeatureFlags::all());
2068+
assert_eq!(version.feature_flags, SUPPORTED_FEATURE_FLAGS);
20652069
}
20662070

20672071
#[async_test]
@@ -2081,7 +2085,7 @@ mod tests {
20812085
interrupt_page_or_target_info: TargetInfo::new()
20822086
.with_sint(2)
20832087
.with_vtl(0)
2084-
.with_feature_flags(FeatureFlags::all().into())
2088+
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
20852089
.into(),
20862090
parent_to_child_monitor_page_gpa: 0,
20872091
child_to_parent_monitor_page_gpa: 0,
@@ -2135,7 +2139,7 @@ mod tests {
21352139
interrupt_page_or_target_info: TargetInfo::new()
21362140
.with_sint(2)
21372141
.with_vtl(0)
2138-
.with_feature_flags(FeatureFlags::all().into())
2142+
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
21392143
.into(),
21402144
parent_to_child_monitor_page_gpa: 0,
21412145
child_to_parent_monitor_page_gpa: 0,
@@ -2162,7 +2166,7 @@ mod tests {
21622166
interrupt_page_or_target_info: TargetInfo::new()
21632167
.with_sint(2)
21642168
.with_vtl(0)
2165-
.with_feature_flags(FeatureFlags::all().into())
2169+
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
21662170
.into(),
21672171
parent_to_child_monitor_page_gpa: 0,
21682172
child_to_parent_monitor_page_gpa: 0,

‎vm/devices/vmbus/vmbus_client/src/saved_state.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use crate::OfferInfo;
55
use crate::RestoreError;
66
use crate::RestoredChannel;
7+
use crate::SUPPORTED_FEATURE_FLAGS;
78
use guid::Guid;
89
use mesh::payload::Protobuf;
910
use vmbus_channel::bus::OfferKey;
@@ -241,7 +242,7 @@ impl TryFrom<ClientState> for super::ClientState {
241242
.ok_or(RestoreError::UnsupportedVersion(version))?;
242243

243244
let feature_flags = FeatureFlags::from(feature_flags);
244-
if feature_flags.contains_unsupported_bits() {
245+
if !SUPPORTED_FEATURE_FLAGS.contains(feature_flags) {
245246
return Err(RestoreError::UnsupportedFeatureFlags(feature_flags.into()));
246247
}
247248

‎vm/devices/vmbus/vmbus_core/src/protocol.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -175,17 +175,9 @@ pub struct FeatureFlags {
175175
}
176176

177177
impl FeatureFlags {
178-
pub const fn all() -> Self {
179-
Self::new()
180-
.with_guest_specified_signal_parameters(true)
181-
.with_channel_interrupt_redirection(true)
182-
.with_modify_connection(true)
183-
.with_client_id(true)
184-
.with_confidential_channels(true)
185-
}
186-
187-
pub fn contains_unsupported_bits(&self) -> bool {
188-
u32::from(*self) & !u32::from(Self::all()) != 0
178+
/// Returns true if `other` contains only flags that are also set in `self`.
179+
pub fn contains(&self, other: FeatureFlags) -> bool {
180+
self.into_bits() & other.into_bits() == other.into_bits()
189181
}
190182
}
191183

‎vm/devices/vmbus/vmbus_server/src/channels.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,6 +1193,14 @@ static SUPPORTED_VERSIONS: &[Version] = &[
11931193
Version::Copper,
11941194
];
11951195

1196+
// Feature flags that are always supported.
1197+
// N.B. Confidential channels are conditionally supported if running in the paravisor.
1198+
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::new()
1199+
.with_guest_specified_signal_parameters(true)
1200+
.with_channel_interrupt_redirection(true)
1201+
.with_modify_connection(true)
1202+
.with_client_id(true);
1203+
11961204
/// Trait for sending requests to devices and the guest.
11971205
pub trait Notifier: Send {
11981206
/// Requests a channel action.
@@ -2233,16 +2241,17 @@ impl<'a, N: 'a + Notifier> ServerWithNotifier<'a, N> {
22332241
// The max version and features may be limited in order to test older protocol versions.
22342242
//
22352243
// N.B. Confidential channels should only be enabled if the connection is trusted.
2244+
let max_supported_flags =
2245+
SUPPORTED_FEATURE_FLAGS.with_confidential_channels(request.trusted);
2246+
22362247
if let Some(max_version) = self.inner.max_version {
22372248
if version as u32 > max_version.version {
22382249
return None;
22392250
}
22402251

2241-
max_version.feature_flags.with_confidential_channels(
2242-
max_version.feature_flags.confidential_channels() && request.trusted,
2243-
)
2252+
max_supported_flags & max_version.feature_flags
22442253
} else {
2245-
FeatureFlags::all().with_confidential_channels(request.trusted)
2254+
max_supported_flags
22462255
}
22472256
} else {
22482257
FeatureFlags::new()
@@ -3917,7 +3926,7 @@ mod tests {
39173926
server.with_notifier(notifier).complete_initiate_contact(
39183927
ModifyConnectionResponse::Supported(
39193928
protocol::ConnectionState::SUCCESSFUL,
3920-
FeatureFlags::all(),
3929+
SUPPORTED_FEATURE_FLAGS,
39213930
),
39223931
);
39233932

@@ -4182,7 +4191,7 @@ mod tests {
41824191
.with_notifier(&mut notifier)
41834192
.complete_initiate_contact(ModifyConnectionResponse::Supported(
41844193
protocol::ConnectionState::SUCCESSFUL,
4185-
FeatureFlags::all(),
4194+
SUPPORTED_FEATURE_FLAGS,
41864195
));
41874196

41884197
let version_response = protocol::VersionResponse {
@@ -4380,7 +4389,7 @@ mod tests {
43804389
.with_notifier(&mut notifier)
43814390
.complete_initiate_contact(ModifyConnectionResponse::Supported(
43824391
protocol::ConnectionState::SUCCESSFUL,
4383-
FeatureFlags::all(),
4392+
SUPPORTED_FEATURE_FLAGS,
43844393
));
43854394

43864395
// Discard the version response message.
@@ -4466,7 +4475,7 @@ mod tests {
44664475
self.c()
44674476
.complete_modify_connection(ModifyConnectionResponse::Supported(
44684477
protocol::ConnectionState::SUCCESSFUL,
4469-
FeatureFlags::all(),
4478+
SUPPORTED_FEATURE_FLAGS,
44704479
));
44714480
}
44724481

@@ -4663,7 +4672,7 @@ mod tests {
46634672
self.c()
46644673
.complete_initiate_contact(ModifyConnectionResponse::Supported(
46654674
protocol::ConnectionState::SUCCESSFUL,
4666-
FeatureFlags::all(),
4675+
SUPPORTED_FEATURE_FLAGS,
46674676
));
46684677

46694678
let version = self.version.unwrap();
@@ -4714,7 +4723,7 @@ mod tests {
47144723
env.c()
47154724
.complete_initiate_contact(ModifyConnectionResponse::Supported(
47164725
protocol::ConnectionState::SUCCESSFUL,
4717-
FeatureFlags::all(),
4726+
SUPPORTED_FEATURE_FLAGS,
47184727
));
47194728
let offer_id3 = env.offer(3);
47204729
env.c().handle_request_offers().unwrap();
@@ -4995,7 +5004,7 @@ mod tests {
49955004
env.c()
49965005
.complete_modify_connection(ModifyConnectionResponse::Supported(
49975006
protocol::ConnectionState::SUCCESSFUL,
4998-
FeatureFlags::all(),
5007+
SUPPORTED_FEATURE_FLAGS,
49995008
));
50005009

50015010
env.notifier
@@ -5153,7 +5162,7 @@ mod tests {
51535162
env.c()
51545163
.complete_modify_connection(ModifyConnectionResponse::Supported(
51555164
protocol::ConnectionState::FAILED_UNKNOWN_FAILURE,
5156-
FeatureFlags::all(),
5165+
SUPPORTED_FEATURE_FLAGS,
51575166
));
51585167

51595168
env.notifier

‎vm/devices/vmbus/vmbus_server/src/channels/saved_state.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use super::OfferError;
55
use super::OfferParamsInternal;
66
use super::OfferedInfo;
77
use super::RestoreState;
8+
use super::SUPPORTED_FEATURE_FLAGS;
89
use guid::Guid;
910
use mesh::payload::Protobuf;
1011
use std::fmt::Display;
@@ -315,15 +316,16 @@ impl VersionInfo {
315316
}
316317
}
317318

318-
fn restore(self) -> Result<vmbus_core::VersionInfo, RestoreError> {
319+
fn restore(self, trusted: bool) -> Result<vmbus_core::VersionInfo, RestoreError> {
319320
let version = super::SUPPORTED_VERSIONS
320321
.iter()
321322
.find(|v| self.version == **v as u32)
322323
.copied()
323324
.ok_or(RestoreError::UnsupportedVersion(self.version))?;
324325

325326
let feature_flags = FeatureFlags::from(self.feature_flags);
326-
if feature_flags.contains_unsupported_bits() {
327+
let supported_flags = SUPPORTED_FEATURE_FLAGS.with_confidential_channels(trusted);
328+
if !supported_flags.contains(feature_flags) {
327329
return Err(RestoreError::UnsupportedFeatureFlags(feature_flags.into()));
328330
}
329331

@@ -429,7 +431,7 @@ impl Connection {
429431
trusted,
430432
} => super::ConnectionState::Connecting {
431433
info: super::ConnectionInfo {
432-
version: version.restore()?,
434+
version: version.restore(trusted)?,
433435
trusted,
434436
interrupt_page,
435437
monitor_page: monitor_page.map(MonitorPageGpas::restore),
@@ -450,7 +452,7 @@ impl Connection {
450452
client_id,
451453
trusted,
452454
} => super::ConnectionState::Connected(super::ConnectionInfo {
453-
version: version.restore()?,
455+
version: version.restore(trusted)?,
454456
trusted,
455457
offers_sent,
456458
interrupt_page,
@@ -805,7 +807,9 @@ impl ReservedState {
805807
}
806808

807809
fn restore(&self) -> Result<super::ReservedState, RestoreError> {
808-
let version = self.version.clone().restore().map_err(|e| match e {
810+
// We don't know if the connection when the channel was reserved was trusted, so assume it
811+
// was for what feature flags are accepted here; it doesn't affect any actual behavior.
812+
let version = self.version.clone().restore(true).map_err(|e| match e {
809813
RestoreError::UnsupportedVersion(v) => RestoreError::UnsupportedReserveVersion(v),
810814
RestoreError::UnsupportedFeatureFlags(f) => {
811815
RestoreError::UnsupportedReserveFeatureFlags(f)

‎vm/devices/vmbus/vmbus_server/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ impl<'a, T: Spawn> VmbusServerBuilder<'a, T> {
444444
.map(|_| {
445445
ModifyConnectionResponse::Supported(
446446
protocol::ConnectionState::SUCCESSFUL,
447-
protocol::FeatureFlags::all(),
447+
protocol::FeatureFlags::from_bits(u32::MAX),
448448
)
449449
})
450450
.boxed()

0 commit comments

Comments
 (0)