Skip to content

Commit 06c69dd

Browse files
committed
fix(gpu): validate exact device requests
Require exact driver GPU device lists to be tied to a GPU request, allow a single exact device to use the default countless request, and require explicit matching counts for multi-device lists. Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent 0a8cbd9 commit 06c69dd

10 files changed

Lines changed: 457 additions & 46 deletions

File tree

crates/openshell-core/src/gpu.rs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,42 @@ pub fn cdi_gpu_device_ids(
6363
}
6464
}
6565

66+
/// Validate that an explicit GPU count matches a driver-owned device list.
67+
///
68+
/// Drivers call this when a request combines portable `gpu.count` with exact
69+
/// device identifiers in `driver_config`.
70+
///
71+
/// # Errors
72+
/// Returns an error when the GPU request is absent or when `gpu.count` does
73+
/// not equal `device_count`. A single exact device is compatible with the
74+
/// default GPU request where `gpu.count` is absent.
75+
pub fn validate_driver_gpu_device_count(
76+
gpu: Option<&DriverGpuResourceRequirements>,
77+
device_count: usize,
78+
driver_config_field: &str,
79+
) -> Result<(), String> {
80+
let Some(gpu) = gpu else {
81+
return Err(format!("{driver_config_field} requires a gpu request"));
82+
};
83+
84+
let Some(count) = gpu.count else {
85+
if device_count == 1 {
86+
return Ok(());
87+
}
88+
return Err(format!(
89+
"{driver_config_field} requires an explicit gpu count matching its length ({device_count})"
90+
));
91+
};
92+
93+
if usize::try_from(count).ok() != Some(device_count) {
94+
return Err(format!(
95+
"gpu count ({count}) must match {driver_config_field} length ({device_count})"
96+
));
97+
}
98+
99+
Ok(())
100+
}
101+
66102
#[cfg(test)]
67103
mod tests {
68104
use super::*;
@@ -100,4 +136,54 @@ mod tests {
100136
])
101137
);
102138
}
139+
140+
#[test]
141+
fn validate_driver_gpu_device_count_accepts_matching_count() {
142+
let gpu = DriverGpuResourceRequirements { count: Some(2) };
143+
144+
validate_driver_gpu_device_count(Some(&gpu), 2, "driver_config.cdi_devices")
145+
.expect("matching count should be accepted");
146+
}
147+
148+
#[test]
149+
fn validate_driver_gpu_device_count_accepts_missing_count_for_one_device() {
150+
let gpu = DriverGpuResourceRequirements { count: None };
151+
152+
validate_driver_gpu_device_count(Some(&gpu), 1, "driver_config.cdi_devices")
153+
.expect("single exact device should be compatible with a default GPU request");
154+
}
155+
156+
#[test]
157+
fn validate_driver_gpu_device_count_rejects_missing_gpu_request() {
158+
let err = validate_driver_gpu_device_count(None, 1, "driver_config.cdi_devices")
159+
.expect_err("missing GPU request should be rejected");
160+
161+
assert_eq!(err, "driver_config.cdi_devices requires a gpu request");
162+
}
163+
164+
#[test]
165+
fn validate_driver_gpu_device_count_rejects_missing_count_for_multiple_devices() {
166+
let gpu = DriverGpuResourceRequirements { count: None };
167+
168+
let err = validate_driver_gpu_device_count(Some(&gpu), 2, "driver_config.cdi_devices")
169+
.expect_err("missing count should be rejected for multiple devices");
170+
171+
assert_eq!(
172+
err,
173+
"driver_config.cdi_devices requires an explicit gpu count matching its length (2)"
174+
);
175+
}
176+
177+
#[test]
178+
fn validate_driver_gpu_device_count_rejects_mismatch() {
179+
let gpu = DriverGpuResourceRequirements { count: Some(2) };
180+
181+
let err = validate_driver_gpu_device_count(Some(&gpu), 1, "driver_config.cdi_devices")
182+
.expect_err("mismatched count should be rejected");
183+
184+
assert_eq!(
185+
err,
186+
"gpu count (2) must match driver_config.cdi_devices length (1)"
187+
);
188+
}
103189
}

crates/openshell-driver-docker/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ contract:
3232
| `apparmor=unconfined` | Avoids Docker's default profile blocking required mount operations. |
3333
| `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. |
3434
| `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. |
35-
| CDI GPU request | Uses `driver_config.cdi_devices` when set; otherwise requests all NVIDIA GPUs when the sandbox spec asks for GPU support and daemon CDI support is detected. Counted GPU requests are rejected. |
35+
| CDI GPU request | Uses `driver_config.cdi_devices` when set; otherwise requests all NVIDIA GPUs when the sandbox spec asks for GPU support and daemon CDI support is detected. Count-only GPU requests are rejected; exact CDI device lists with more than one entry require an explicit GPU count matching the device list length. |
3636

3737
The agent child process does not retain these supervisor privileges.
3838

crates/openshell-driver-docker/src/lib.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ use openshell_core::driver_utils::{
2727
LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME,
2828
LABEL_SANDBOX_NAMESPACE, SUPERVISOR_IMAGE_BINARY_PATH, supervisor_image_should_refresh,
2929
};
30-
use openshell_core::gpu::{cdi_gpu_device_ids, driver_gpu_requirements};
30+
use openshell_core::gpu::{
31+
cdi_gpu_device_ids, driver_gpu_requirements, validate_driver_gpu_device_count,
32+
};
3133
use openshell_core::progress::{
3234
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
3335
format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail,
@@ -514,13 +516,14 @@ impl DockerComputeDriver {
514516
supports_gpu: bool,
515517
driver_config: &DockerSandboxDriverConfig,
516518
) -> Result<(), Status> {
517-
if gpu_requirements.is_none() && driver_config.cdi_devices.is_some() {
518-
return Err(Status::invalid_argument(
519-
"driver_config.cdi_devices requires gpu=true",
520-
));
521-
}
522-
523-
if gpu_requirements.and_then(|gpu| gpu.count).is_some() {
519+
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
520+
validate_driver_gpu_device_count(
521+
gpu_requirements,
522+
cdi_devices.len(),
523+
"driver_config.cdi_devices",
524+
)
525+
.map_err(Status::invalid_argument)?;
526+
} else if gpu_requirements.and_then(|gpu| gpu.count).is_some() {
524527
return Err(Status::invalid_argument(
525528
"docker GPU count requests are not supported; use --gpu without a count or driver_config.cdi_devices",
526529
));
@@ -2129,10 +2132,13 @@ fn build_device_requests(sandbox: &DriverSandbox) -> Result<Option<Vec<DeviceReq
21292132
.cdi_devices
21302133
.unwrap_or_default();
21312134
let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref());
2132-
if gpu_requirements.is_none() && !cdi_devices.is_empty() {
2133-
return Err(Status::invalid_argument(
2134-
"driver_config.cdi_devices requires gpu=true",
2135-
));
2135+
if !cdi_devices.is_empty() {
2136+
validate_driver_gpu_device_count(
2137+
gpu_requirements,
2138+
cdi_devices.len(),
2139+
"driver_config.cdi_devices",
2140+
)
2141+
.map_err(Status::invalid_argument)?;
21362142
}
21372143

21382144
Ok(

crates/openshell-driver-docker/src/tests.rs

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,94 @@ fn validate_sandbox_rejects_gpu_count_request() {
10681068
);
10691069
}
10701070

1071+
#[test]
1072+
fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() {
1073+
let mut config = runtime_config();
1074+
config.supports_gpu = true;
1075+
let mut sandbox = test_sandbox();
1076+
let spec = sandbox.spec.as_mut().unwrap();
1077+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1078+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[
1079+
"nvidia.com/gpu=0",
1080+
"nvidia.com/gpu=1",
1081+
]));
1082+
1083+
DockerComputeDriver::validate_sandbox(&sandbox, &config)
1084+
.expect("matching explicit CDI device count should be accepted");
1085+
}
1086+
1087+
#[test]
1088+
fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() {
1089+
let mut config = runtime_config();
1090+
config.supports_gpu = true;
1091+
let mut sandbox = test_sandbox();
1092+
let spec = sandbox.spec.as_mut().unwrap();
1093+
spec.resource_requirements = Some(gpu_resources(None));
1094+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1095+
1096+
DockerComputeDriver::validate_sandbox(&sandbox, &config)
1097+
.expect("single exact CDI device should be compatible with a default GPU request");
1098+
}
1099+
1100+
#[test]
1101+
fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() {
1102+
let mut config = runtime_config();
1103+
config.supports_gpu = true;
1104+
let mut sandbox = test_sandbox();
1105+
let spec = sandbox.spec.as_mut().unwrap();
1106+
spec.resource_requirements = Some(gpu_resources(None));
1107+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[
1108+
"nvidia.com/gpu=0",
1109+
"nvidia.com/gpu=1",
1110+
]));
1111+
1112+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1113+
1114+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1115+
assert!(
1116+
err.message()
1117+
.contains("requires an explicit gpu count matching its length (2)")
1118+
);
1119+
}
1120+
1121+
#[test]
1122+
fn validate_sandbox_rejects_cdi_devices_without_gpu_request() {
1123+
let mut config = runtime_config();
1124+
config.supports_gpu = true;
1125+
let mut sandbox = test_sandbox();
1126+
sandbox
1127+
.spec
1128+
.as_mut()
1129+
.unwrap()
1130+
.template
1131+
.as_mut()
1132+
.unwrap()
1133+
.driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1134+
1135+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1136+
1137+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1138+
assert!(err.message().contains("requires a gpu request"));
1139+
}
1140+
1141+
#[test]
1142+
fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() {
1143+
let mut config = runtime_config();
1144+
config.supports_gpu = true;
1145+
let mut sandbox = test_sandbox();
1146+
let spec = sandbox.spec.as_mut().unwrap();
1147+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1148+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1149+
1150+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1151+
1152+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1153+
assert!(
1154+
err.message()
1155+
.contains("gpu count (2) must match driver_config.cdi_devices length (1)")
1156+
);
1157+
}
1158+
10711159
#[test]
10721160
fn validate_sandbox_rejects_template_errors_before_device_config() {
10731161
let config = runtime_config();
@@ -1153,7 +1241,25 @@ fn build_container_create_body_passes_explicit_cdi_device_id_through() {
11531241
}
11541242

11551243
#[test]
1156-
fn build_container_create_body_rejects_cdi_devices_without_gpu() {
1244+
fn build_container_create_body_rejects_gpu_count_mismatched_cdi_devices() {
1245+
let mut config = runtime_config();
1246+
config.supports_gpu = true;
1247+
let mut sandbox = test_sandbox();
1248+
let spec = sandbox.spec.as_mut().unwrap();
1249+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1250+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1251+
1252+
let err = build_container_create_body(&sandbox, &config).unwrap_err();
1253+
1254+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1255+
assert!(
1256+
err.message()
1257+
.contains("gpu count (2) must match driver_config.cdi_devices length (1)")
1258+
);
1259+
}
1260+
1261+
#[test]
1262+
fn build_container_create_body_rejects_cdi_devices_without_gpu_request() {
11571263
let mut sandbox = test_sandbox();
11581264
sandbox
11591265
.spec
@@ -1166,7 +1272,7 @@ fn build_container_create_body_rejects_cdi_devices_without_gpu() {
11661272

11671273
let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err();
11681274
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1169-
assert!(err.message().contains("requires gpu=true"));
1275+
assert!(err.message().contains("requires a gpu request"));
11701276
}
11711277

11721278
#[test]

crates/openshell-driver-podman/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ The container spec in `container.rs` sets these security-critical fields:
4646
| `no_new_privileges` | `true` | Prevents privilege escalation after exec. |
4747
| `seccomp_profile_path` | `unconfined` | The supervisor installs its own policy-aware BPF filter. A container-level profile can block Landlock/seccomp syscalls during setup. |
4848
| `mounts` | Private tmpfs at `/run/netns` | Lets the supervisor create named network namespaces in rootless Podman. |
49-
| CDI GPU devices | `driver_config.cdi_devices` when set, otherwise all NVIDIA GPUs | Exposes requested GPUs to GPU-enabled sandbox containers. Counted GPU requests are rejected. |
49+
| CDI GPU devices | `driver_config.cdi_devices` when set, otherwise all NVIDIA GPUs | Exposes requested GPUs to GPU-enabled sandbox containers. Count-only GPU requests are rejected; exact CDI device lists with more than one entry require an explicit GPU count matching the device list length. |
5050

5151
The restricted agent child does not retain these supervisor privileges.
5252

crates/openshell-driver-podman/src/container.rs

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
66
use crate::config::PodmanComputeConfig;
77
use openshell_core::ComputeDriverError;
8-
use openshell_core::gpu::{cdi_gpu_device_ids, driver_gpu_requirements};
8+
use openshell_core::gpu::{
9+
cdi_gpu_device_ids, driver_gpu_requirements, validate_driver_gpu_device_count,
10+
};
911
use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate};
1012
use openshell_core::proto_struct::deserialize_optional_non_empty_string_list;
1113
use openshell_core::{driver_mounts, proto_struct};
@@ -485,10 +487,13 @@ fn build_devices(sandbox: &DriverSandbox) -> Result<Option<Vec<LinuxDevice>>, Co
485487
.cdi_devices
486488
.unwrap_or_default();
487489
let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref());
488-
if gpu_requirements.is_none() && !cdi_devices.is_empty() {
489-
return Err(ComputeDriverError::InvalidArgument(
490-
"driver_config.cdi_devices requires gpu=true".to_string(),
491-
));
490+
if !cdi_devices.is_empty() {
491+
validate_driver_gpu_device_count(
492+
gpu_requirements,
493+
cdi_devices.len(),
494+
"driver_config.cdi_devices",
495+
)
496+
.map_err(ComputeDriverError::InvalidArgument)?;
492497
}
493498

494499
Ok(
@@ -1289,7 +1294,60 @@ mod tests {
12891294
}
12901295

12911296
#[test]
1292-
fn container_spec_rejects_cdi_devices_without_gpu() {
1297+
fn container_spec_accepts_gpu_count_matching_cdi_devices() {
1298+
use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate};
1299+
1300+
let mut sandbox = test_sandbox("test-id", "test-name");
1301+
sandbox.spec = Some(DriverSandboxSpec {
1302+
resource_requirements: Some(gpu_resources(Some(2))),
1303+
template: Some(DriverSandboxTemplate {
1304+
driver_config: Some(cdi_devices_config(&[
1305+
"nvidia.com/gpu=0",
1306+
"nvidia.com/gpu=1",
1307+
])),
1308+
..Default::default()
1309+
}),
1310+
..Default::default()
1311+
});
1312+
let config = test_config();
1313+
let spec = build_container_spec(&sandbox, &config);
1314+
1315+
assert_eq!(spec["devices"].as_array().map(Vec::len), Some(2));
1316+
assert_eq!(
1317+
spec["devices"][0]["path"].as_str(),
1318+
Some("nvidia.com/gpu=0")
1319+
);
1320+
assert_eq!(
1321+
spec["devices"][1]["path"].as_str(),
1322+
Some("nvidia.com/gpu=1")
1323+
);
1324+
}
1325+
1326+
#[test]
1327+
fn container_spec_rejects_gpu_count_mismatched_cdi_devices() {
1328+
use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate};
1329+
1330+
let mut sandbox = test_sandbox("test-id", "test-name");
1331+
sandbox.spec = Some(DriverSandboxSpec {
1332+
resource_requirements: Some(gpu_resources(Some(2))),
1333+
template: Some(DriverSandboxTemplate {
1334+
driver_config: Some(cdi_devices_config(&["nvidia.com/gpu=0"])),
1335+
..Default::default()
1336+
}),
1337+
..Default::default()
1338+
});
1339+
let config = test_config();
1340+
1341+
let err = try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err();
1342+
assert!(matches!(err, ComputeDriverError::InvalidArgument(_)));
1343+
assert!(
1344+
err.to_string()
1345+
.contains("gpu count (2) must match driver_config.cdi_devices length (1)")
1346+
);
1347+
}
1348+
1349+
#[test]
1350+
fn container_spec_rejects_cdi_devices_without_gpu_request() {
12931351
use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate};
12941352

12951353
let mut sandbox = test_sandbox("test-id", "test-name");
@@ -1304,7 +1362,7 @@ mod tests {
13041362

13051363
let err = try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err();
13061364
assert!(matches!(err, ComputeDriverError::InvalidArgument(_)));
1307-
assert!(err.to_string().contains("requires gpu=true"));
1365+
assert!(err.to_string().contains("requires a gpu request"));
13081366
}
13091367

13101368
#[test]

0 commit comments

Comments
 (0)